From 3d33935fbacca01ae639a19cc2eba99c75ed6541 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 9 Sep 2026 17:07:51 -0600 Subject: [PATCH] Route Docker sandboxes through the sandbox driver Fabro's Docker provider kind now maps onto the sandbox-driver Docker provider instead of its own bollard implementation. Fabro keeps what is its own: the clone decision and repository layout, the GitHub App push credentials embedded in origin, git retry classification, the managed labels that gate destructive operations, and the run-facing events. - `clone.rs` performs fabro's clone over the driver `Git` and `Exec` facets. An exact commit goes through the driver's pinned clone; a tag pin runs fabro's init/fetch/attach sequence through `Exec` so the fully qualified tag ref is the only revision consulted. Network failures retry through `git_retry`, which now classifies driver errors (exec output, provider retryability) and never replays an operation whose outcome is unknown. - `DriverSandbox` gains a pending-create state, a `RepoWorkspace` with push-credential state, path resolution against the cloned working directory, git lifecycle methods, image-pull progress mapped from driver events, and an embedded terminal over the driver `Pty` facet. - `docker.rs` builds the driver `SandboxSpec` (image, `/workspace`, fabro labels, env, cpu/memory, network policy, run name) and attaches by persisted container id, refusing containers without fabro's labels. - Inventory, details, diagnostics, terminal, and reconnect run over the driver: a `DriverInventoryProvider` lists by fabro's managed label and projects `SandboxStatus` into fabro's inventory and details shapes. - The bollard-based `docker.rs`, `provider/docker.rs`, Docker terminal, Docker error variants, the `docker` cargo feature, and the bollard and tar dependencies are gone. The Docker integration tests, the agent shell test, the workflow artifact test, and the driver benchmark run against the driver-backed sandbox. Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 2 - Cargo.toml | 2 - docs/public/reference/sdk.mdx | 4 +- lib/apps/fabro-server/Cargo.toml | 2 +- lib/apps/fabro-server/src/diagnostics.rs | 4 +- lib/apps/fabro-server/src/server.rs | 15 +- lib/components/fabro-agent/Cargo.toml | 2 - .../fabro-agent/src/docker_sandbox.rs | 2 - lib/components/fabro-agent/src/lib.rs | 6 +- .../fabro-agent/tests/it/docker_shell.rs | 8 +- lib/components/fabro-agent/tests/it/main.rs | 1 - lib/components/fabro-sandbox/Cargo.toml | 5 - lib/components/fabro-sandbox/src/clone.rs | 430 +++ .../fabro-sandbox/src/clone_source.rs | 5 - lib/components/fabro-sandbox/src/details.rs | 606 +--- lib/components/fabro-sandbox/src/docker.rs | 3193 ++--------------- .../fabro-sandbox/src/driver_sandbox.rs | 683 +++- lib/components/fabro-sandbox/src/error.rs | 46 - lib/components/fabro-sandbox/src/exec.rs | 23 +- .../fabro-sandbox/src/from_environment.rs | 5 - lib/components/fabro-sandbox/src/git_retry.rs | 36 + lib/components/fabro-sandbox/src/lib.rs | 16 +- .../fabro-sandbox/src/managed_labels.rs | 2 - lib/components/fabro-sandbox/src/provider.rs | 40 +- .../fabro-sandbox/src/provider/daytona.rs | 41 +- .../fabro-sandbox/src/provider/docker.rs | 179 - .../fabro-sandbox/src/provider/driver.rs | 246 ++ lib/components/fabro-sandbox/src/reconnect.rs | 10 +- lib/components/fabro-sandbox/src/sandbox.rs | 42 +- .../fabro-sandbox/src/sandbox_spec.rs | 23 +- lib/components/fabro-sandbox/src/terminal.rs | 277 +- .../fabro-sandbox/src/test_support.rs | 6 +- .../fabro-sandbox/tests/docker_streaming.rs | 66 +- .../fabro-sandbox/tests/driver_bench.rs | 34 +- lib/components/fabro-sandbox/tests/error.rs | 21 - lib/components/fabro-workflow/Cargo.toml | 2 +- .../fabro-workflow/tests/it/integration.rs | 3 +- 37 files changed, 1875 insertions(+), 4213 deletions(-) delete mode 100644 lib/components/fabro-agent/src/docker_sandbox.rs create mode 100644 lib/components/fabro-sandbox/src/clone.rs delete mode 100644 lib/components/fabro-sandbox/src/provider/docker.rs create mode 100644 lib/components/fabro-sandbox/src/provider/driver.rs diff --git a/Cargo.lock b/Cargo.lock index 229c303d5..eb5a02dbb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2984,7 +2984,6 @@ dependencies = [ "anyhow", "async-trait", "base64", - "bollard", "chrono", "daytona-api-client", "daytona-sdk", @@ -3018,7 +3017,6 @@ dependencies = [ "serde_json", "sha2 0.10.9", "strum 0.28.0", - "tar", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 6e27bed20..af72dc197 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,8 +62,6 @@ clap_complete = "4" jsonschema = { version = "0.42", default-features = false } chrono = { version = "0.4", features = ["clock", "serde"] } dashmap = "6" -bollard = "0.18" -tar = "0.4" cli-table = { version = "0.5", default-features = false } console = "0.15" dialoguer = "0.12" diff --git a/docs/public/reference/sdk.mdx b/docs/public/reference/sdk.mdx index 12dee0001..614e2e063 100644 --- a/docs/public/reference/sdk.mdx +++ b/docs/public/reference/sdk.mdx @@ -159,8 +159,8 @@ pub trait Sandbox: Send + Sync { | Type | Description | |---|---| -| `LocalSandbox` | Executes directly on the local filesystem. | -| `DockerSandbox` | Runs inside a Docker container (feature-gated: `docker`). | +| `local_sandbox(...)` | Executes directly on the local filesystem through the sandbox driver Host provider. | +| `docker_sandbox(...)` | Runs inside a Docker container through the sandbox driver. | The `DaytonaSandbox` implementation (feature-gated: `daytona`) runs inside a Daytona cloud sandbox. diff --git a/lib/apps/fabro-server/Cargo.toml b/lib/apps/fabro-server/Cargo.toml index ab4475c15..d21d7c515 100644 --- a/lib/apps/fabro-server/Cargo.toml +++ b/lib/apps/fabro-server/Cargo.toml @@ -34,7 +34,7 @@ fabro-slack = { path = "../../components/fabro-slack" } fabro-workflow = { path = "../../components/fabro-workflow" } fabro-workflow-version = { path = "../../components/fabro-workflow-version" } fabro-validate = { path = "../../components/fabro-validate" } -fabro-sandbox = { path = "../../components/fabro-sandbox", features = ["daytona", "docker"] } +fabro-sandbox = { path = "../../components/fabro-sandbox", features = ["daytona"] } fabro-github = { path = "../../components/fabro-github" } fabro-agent = { path = "../../components/fabro-agent" } fabro-llm = { path = "../../components/fabro-llm" } diff --git a/lib/apps/fabro-server/src/diagnostics.rs b/lib/apps/fabro-server/src/diagnostics.rs index 9a25e4d97..1e1add25e 100644 --- a/lib/apps/fabro-server/src/diagnostics.rs +++ b/lib/apps/fabro-server/src/diagnostics.rs @@ -10,7 +10,7 @@ use fabro_llm::client::Client as LlmClient; use fabro_llm::model_test::{ModelTestStatus, run_basic_model_probe_with_timeout}; use fabro_model::{Catalog, ProviderId}; use fabro_redact::redact_string; -use fabro_sandbox::{DockerSandboxProvider, daytona}; +use fabro_sandbox::daytona; use fabro_static::EnvVars; use fabro_types::SandboxProviderKind; use fabro_types::settings::ServerAuthMethod; @@ -584,7 +584,7 @@ async fn check_docker_sandbox(state: &AppState) -> CheckResult { .providers .is_enabled(&SandboxProviderKind::DOCKER), || async { - DockerSandboxProvider::check_daemon() + fabro_sandbox::check_docker_daemon() .await .map_err(|err| err.display_with_causes()) }, diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 75ff2b8a8..6f8778461 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -69,10 +69,11 @@ use fabro_model::{BilledTokenCounts, Catalog, ModelRef, ModelTestMode, ProviderI use fabro_redact::redact_jsonl_line; use fabro_sandbox::daytona::{self, DaytonaSandbox}; use fabro_sandbox::details::sandbox_details; +use fabro_sandbox::driver::ProviderConnectOptions; use fabro_sandbox::reconnect::reconnect_for_run; use fabro_sandbox::{ - DaytonaSandboxProvider, DockerSandboxProvider, LocalSandboxProvider, Sandbox, SandboxProvider, - SandboxProviderRegistry, + DaytonaSandboxProvider, DriverInventoryProvider, LocalSandboxProvider, Sandbox, + SandboxProvider, SandboxProviderRegistry, }; use fabro_slack::client::{PostedMessage as SlackPostedMessage, SlackClient}; use fabro_slack::config::{ @@ -2340,8 +2341,14 @@ fn build_sandbox_provider_registry( providers.push(Arc::new(LocalSandboxProvider)); } - if provider_settings.is_enabled(&SandboxProviderKind::DOCKER) { - providers.push(Arc::new(DockerSandboxProvider::new())); + if let Some(docker) = provider_settings.get(&SandboxProviderKind::DOCKER) { + if docker.enabled { + providers.push(Arc::new(DriverInventoryProvider::lazy( + SandboxProviderKind::DOCKER, + docker.clone(), + ProviderConnectOptions::default(), + ))); + } } if provider_settings.is_enabled(&SandboxProviderKind::DAYTONA) && daytona_api_key.is_some() { diff --git a/lib/components/fabro-agent/Cargo.toml b/lib/components/fabro-agent/Cargo.toml index 91f2e0e98..3b3b0a0a6 100644 --- a/lib/components/fabro-agent/Cargo.toml +++ b/lib/components/fabro-agent/Cargo.toml @@ -11,8 +11,6 @@ keywords = ["llm", "ai", "agent", "coding"] categories = ["api-bindings"] [features] -default = ["docker"] -docker = ["fabro-sandbox/docker"] quarantine = [] [lib] diff --git a/lib/components/fabro-agent/src/docker_sandbox.rs b/lib/components/fabro-agent/src/docker_sandbox.rs deleted file mode 100644 index 2c3f4f257..000000000 --- a/lib/components/fabro-agent/src/docker_sandbox.rs +++ /dev/null @@ -1,2 +0,0 @@ -// Re-export from fabro-sandbox -pub use fabro_sandbox::docker::{DockerSandbox, DockerSandboxOptions}; diff --git a/lib/components/fabro-agent/src/lib.rs b/lib/components/fabro-agent/src/lib.rs index 88e5c7965..05867e821 100644 --- a/lib/components/fabro-agent/src/lib.rs +++ b/lib/components/fabro-agent/src/lib.rs @@ -1,6 +1,3 @@ -#[cfg(feature = "docker")] -pub mod docker_sandbox; - pub mod agent_profile; pub mod apply_patch; pub mod cli; @@ -38,11 +35,10 @@ pub use config::{ NativeToolOptions, SessionOptions, ToolAccess, ToolAccessPolicy, ToolApprovalAdapter, ToolExposureMode, ToolHookCallback, ToolHookDecision, ToolSecrets, }; -#[cfg(feature = "docker")] -pub use docker_sandbox::{DockerSandbox, DockerSandboxOptions}; pub use error::{CompactionError, Error, InterruptReason, Result}; pub use event::Emitter; pub use fabro_mcp::config::McpServerSettings; +pub use fabro_sandbox::{DockerSandboxOptions, docker_sandbox}; pub use fabro_types::SteeringMessage; pub use history::History; pub use local_sandbox::{DriverSandbox, local_sandbox}; diff --git a/lib/components/fabro-agent/tests/it/docker_shell.rs b/lib/components/fabro-agent/tests/it/docker_shell.rs index 83ad3ac0a..dc0c9502d 100644 --- a/lib/components/fabro-agent/tests/it/docker_shell.rs +++ b/lib/components/fabro-agent/tests/it/docker_shell.rs @@ -9,7 +9,7 @@ use fabro_agent::sandbox::Sandbox; use fabro_agent::tool_registry::ToolContext; use fabro_agent::tools::make_shell_tool; use fabro_agent::types::AgentEvent; -use fabro_agent::{DockerSandbox, DockerSandboxOptions, Emitter}; +use fabro_agent::{DockerSandboxOptions, Emitter, docker_sandbox}; use fabro_types::CommandTermination; use tokio::sync::broadcast; use tokio_util::sync::CancellationToken; @@ -17,7 +17,7 @@ use tokio_util::sync::CancellationToken; #[tokio::test] #[ignore = "requires real Docker container lifecycle; run explicitly when changing shell tool exec integration"] async fn shell_reports_real_docker_process_outcome() { - let Ok(sandbox) = DockerSandbox::new( + let Ok(sandbox) = docker_sandbox( DockerSandboxOptions { image: "buildpack-deps:noble".to_string(), auto_pull: false, @@ -30,7 +30,9 @@ async fn shell_reports_real_docker_process_outcome() { None, None, None, - ) else { + ) + .await + else { return; }; // No Docker daemon or no local image: the integration precondition is not met. diff --git a/lib/components/fabro-agent/tests/it/main.rs b/lib/components/fabro-agent/tests/it/main.rs index 485fe7037..71e1147c9 100644 --- a/lib/components/fabro-agent/tests/it/main.rs +++ b/lib/components/fabro-agent/tests/it/main.rs @@ -1,5 +1,4 @@ mod compaction; -#[cfg(feature = "docker")] mod docker_shell; mod guardrails; mod parity_matrix; diff --git a/lib/components/fabro-sandbox/Cargo.toml b/lib/components/fabro-sandbox/Cargo.toml index 8b8fb303d..dde9d5471 100644 --- a/lib/components/fabro-sandbox/Cargo.toml +++ b/lib/components/fabro-sandbox/Cargo.toml @@ -9,7 +9,6 @@ description = "Sandbox trait and implementations for Fabro agent execution envir [features] default = ["local"] local = [] -docker = ["dep:bollard", "dep:tar"] daytona = ["dep:daytona-sdk", "dep:daytona-api-client", "dep:git2", "dep:fabro-config", "dep:fabro-http", "dep:reqwest-middleware", "dep:rand", "dep:tokio-tungstenite", "dep:futures-util", "dep:rustls"] test-support = [] @@ -58,10 +57,6 @@ fabro-redact.workspace = true futures = { workspace = true } -# docker -bollard = { workspace = true, optional = true } -tar = { workspace = true, optional = true } - # daytona fabro-config = { path = "../../foundation/fabro-config", optional = true } fabro-github = { path = "../fabro-github" } diff --git a/lib/components/fabro-sandbox/src/clone.rs b/lib/components/fabro-sandbox/src/clone.rs new file mode 100644 index 000000000..c81653157 --- /dev/null +++ b/lib/components/fabro-sandbox/src/clone.rs @@ -0,0 +1,430 @@ +//! Fabro's clone orchestration over the sandbox-driver [`Git`] and [`Exec`] +//! facets. +//! +//! The driver clones; fabro decides what to clone, where it lands, which +//! credentials it carries, and how failures retry. The layout is fabro's: +//! the repository checks out under `//` and the +//! run works in `/`, a symlink to the checkout. An +//! exact commit goes through the driver's pinned clone; a tag pin runs +//! fabro's own init, fetch, and attach sequence through `Exec`, because a +//! tag must be fetched by its fully qualified ref so a same-named branch is +//! never consulted. Neither path ever falls back to the branch head. + +use std::time::Duration; + +use fabro_github::token_source::ResolvedToken; +use fabro_redact::DisplaySafeUrl; +use fabro_types::SandboxProviderKind; +use sandbox_driver::{Git as _, GitCloneOptions, GitCredentials, Sandbox as DriverHandle}; +use tokio::time; + +use crate::clone_source::{self, GitHubRepoLayout, PinnedRevision}; +use crate::exec::SandboxExec; +use crate::git_retry::{self, CredentialContext, GitRetryReason, RetryPlan}; +use crate::push_credentials::PushCredentialState; +use crate::redact::redact_auth_url; +use crate::sandbox::shell_quote; +use crate::{ExecResult, ExecStreamingRequest}; + +/// Whole-clone budget, shared by every network and local step. +pub(crate) const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(5); +const STEP_TIMEOUT: Duration = Duration::from_secs(10); + +/// A GitHub clone fabro decided to perform. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct GitHubClone { + pub(crate) origin_url: String, + pub(crate) branch: Option, + pub(crate) tag: Option, + pub(crate) commit_sha: Option, + pub(crate) depth: Option, +} + +/// What the clone left behind: the layout and the token now embedded in +/// `origin`, if any. +pub(crate) struct CloneOutcome { + pub(crate) layout: GitHubRepoLayout, +} + +/// Whether a failing git step talked to the remote. Local steps cannot fail +/// on credentials, so they must not suggest reconfiguring the GitHub App. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum CloneStep { + Network, + Local, +} + +struct CloneFailure { + error: crate::Error, + retry_reason: Option, +} + +/// Clone `plan` into `handle`, laid out under `workspace_root` and +/// `repos_root`, embedding a GitHub App token from `credentials` when one +/// is available. +pub(crate) async fn clone_github_repo( + kind: &SandboxProviderKind, + handle: &dyn DriverHandle, + exec: &SandboxExec<'_>, + plan: &GitHubClone, + workspace_root: &str, + repos_root: &str, + credentials: &PushCredentialState, +) -> crate::Result { + verify_git_available(exec).await?; + let layout = clone_source::github_repo_layout(&plan.origin_url, workspace_root, repos_root)?; + // The clone mints its own token (never a warm-cache reuse) and seeds the + // shared source, so the first refresh compares against the clone token + // instead of believing nothing was ever embedded. + let resolved_token = match credentials.source() { + Some(source) => Some(source.mint_for_clone().await.map_err(|err| { + crate::Error::context_anyhow("Failed to get GitHub App credentials for clone", err) + })?), + None => None, + }; + let credential_context = + CredentialContext::from_snapshot(resolved_token.as_ref().map(|token| &token.snapshot)); + let auth_url = match &resolved_token { + Some(token) => Some( + fabro_github::embed_token_in_url(&plan.origin_url, token.token.expose()).map_err( + |err| { + crate::Error::context_anyhow( + "Failed to build authenticated GitHub clone URL", + err, + ) + }, + )?, + ), + None => None, + }; + + let fs = handle.fs(); + for dir in [workspace_root, layout.repos_owner_path.as_str()] { + fs.create_dir(dir) + .await + .map_err(|error| crate::Error::context(format!("Failed to create {dir}"), error))?; + } + + let deadline = time::Instant::now() + GIT_CLONE_TIMEOUT; + let has_app = credentials.source().is_some(); + match PinnedRevision::from_selectors(plan.tag.as_deref(), plan.commit_sha.as_deref()) { + Some(PinnedRevision::Tag(tag)) => { + // `decide_clone` already requires a branch for a pin; the branch + // names the checkout the run works on. + let branch = plan + .branch + .as_deref() + .filter(|branch| !branch.trim().is_empty()) + .ok_or_else(|| { + crate::Error::message("Tag checkout requires a repository branch") + })?; + clone_pinned_tag( + kind, + exec, + &layout, + plan, + &tag, + branch, + auth_url.as_ref(), + credential_context, + deadline, + has_app, + ) + .await?; + } + pin => { + let git = handle.git().ok_or_else(|| { + crate::Error::message(format!( + "sandbox provider `{kind}` does not support git operations" + )) + })?; + let mut options = GitCloneOptions::default(); + options.branch = plan + .branch + .clone() + .filter(|branch| !branch.trim().is_empty()); + options.commit = plan.commit_sha.clone(); + options.depth = plan.depth; + options.credentials = resolved_token + .as_ref() + .map(|token| GitCredentials::new("x-access-token", token.token.expose())); + let retry_plan = RetryPlan::clone_default(Some(deadline)); + let target = layout.primary_repo_path.clone(); + git_retry::retry_git_operation( + kind.clone(), + "clone", + &retry_plan, + |_attempt| { + let options = options.clone(); + let target = target.clone(); + let origin_url = plan.origin_url.clone(); + let git = &git; + async move { + git.clone_repo(&origin_url, &target, &options) + .await + .map_err(|error| CloneFailure { + retry_reason: git_retry::classify_driver_failure( + &error, + credential_context, + ), + error: clone_failure_error( + crate::Error::driver_error(error), + CloneStep::Network, + has_app, + ), + }) + } + }, + |failure: &CloneFailure| failure.retry_reason, + ) + .await + .map_err(|failure| failure.error)?; + if let Some(pin) = pin { + let head = run_local_step( + exec, + &clone_source::exact_head_revision_command(&layout.primary_repo_path), + "git rev-parse HEAD (pinned checkout)", + deadline, + auth_url.as_ref(), + has_app, + ) + .await?; + pin.verify_head(&head.stdout)?; + } + } + } + + run_local_step( + exec, + &clone_source::repo_symlink_command(&layout), + "create workspace repo symlink", + deadline, + auth_url.as_ref(), + has_app, + ) + .await?; + + if let Some(token) = resolved_token { + embed_origin_credentials(exec, &layout, auth_url.as_ref(), token, credentials).await; + } + Ok(CloneOutcome { layout }) +} + +/// Fabro's exact tag checkout: init, fetch the fully qualified tag ref at +/// the same depth a branch clone gets, attach the admitted branch to the +/// fetched commit, and verify HEAD. Every step runs through `Exec`. +#[expect( + clippy::too_many_arguments, + reason = "the pinned path threads clone inputs, credentials, and the shared deadline" +)] +async fn clone_pinned_tag( + kind: &SandboxProviderKind, + exec: &SandboxExec<'_>, + layout: &GitHubRepoLayout, + plan: &GitHubClone, + tag: &str, + branch: &str, + auth_url: Option<&DisplaySafeUrl>, + credential_context: CredentialContext, + deadline: time::Instant, + has_app: bool, +) -> crate::Result<()> { + let clone_url = auth_url.map_or(plan.origin_url.as_str(), |url| url.as_raw_url().as_str()); + let init = clone_source::exact_repository_init_command(clone_url, &layout.primary_repo_path); + run_local_step( + exec, + &init, + "initialize pinned repository checkout", + deadline, + auth_url, + has_app, + ) + .await?; + + let pin = PinnedRevision::Tag(tag.to_string()); + let fetch = clone_source::pinned_fetch_command( + &layout.primary_repo_path, + "origin", + &pin.fetch_refspec(), + plan.depth.map(|depth| depth as usize), + ); + let retry_plan = RetryPlan::clone_default(Some(deadline)); + git_retry::retry_git_operation( + kind.clone(), + "fetch", + &retry_plan, + |_attempt| async { + let remaining = deadline.saturating_duration_since(time::Instant::now()); + if remaining.is_zero() { + return Err(CloneFailure { + error: crate::Error::message( + "git fetch pinned revision deadline expired before retry", + ), + retry_reason: None, + }); + } + let result = exec + .run_streaming(ExecStreamingRequest { + timeout_ms: Some(millis(remaining)), + working_dir: Some("/"), + ..ExecStreamingRequest::new(&fetch) + }) + .await + .map_err(|error| CloneFailure { + error: crate::Error::context( + "git fetch pinned revision transport failed", + error, + ), + retry_reason: None, + })? + .result; + if result.is_success() { + return Ok(()); + } + let retry_reason = + git_retry::classify_output(&result.stderr, &result.stdout, credential_context) + .retry_reason(); + Err(CloneFailure { + error: clone_failure_error( + result.into_exec_error_with_redactor("git fetch pinned revision", |output| { + redact_auth_url(output, auth_url) + }), + CloneStep::Network, + has_app, + ), + retry_reason, + }) + }, + |failure: &CloneFailure| failure.retry_reason, + ) + .await + .map_err(|failure| failure.error)?; + + let checkout = clone_source::exact_checkout_verify_command( + &layout.primary_repo_path, + branch, + clone_source::FETCH_HEAD_COMMIT, + ); + let head = run_local_step( + exec, + &checkout, + "git checkout pinned revision", + deadline, + auth_url, + has_app, + ) + .await?; + pin.verify_head(&head.stdout)?; + Ok(()) +} + +async fn verify_git_available(exec: &SandboxExec<'_>) -> crate::Result<()> { + let result = exec + .run("git --version", Some(STEP_TIMEOUT), Some("/"), None, None) + .await?; + if !result.is_success() { + return Err(crate::Error::message( + "The sandbox image must include git for repository clone and git lifecycle \ + operations. Use an image with bash and git, such as buildpack-deps:noble.", + )); + } + Ok(()) +} + +/// Run a local (non-network) step under the shared clone deadline. +/// +/// Materializing a large working tree takes far longer than the short fixed +/// timeout used for trivial commands, so these steps get the same budget the +/// network steps have. +async fn run_local_step( + exec: &SandboxExec<'_>, + command: &str, + label: &'static str, + deadline: time::Instant, + auth_url: Option<&DisplaySafeUrl>, + has_app: bool, +) -> crate::Result { + let remaining = deadline.saturating_duration_since(time::Instant::now()); + if remaining.is_zero() { + return Err(crate::Error::message(format!( + "{label} deadline expired before the step could run" + ))); + } + let result = exec + .run(command, Some(remaining), Some("/"), None, None) + .await + .map_err(|error| crate::Error::context(format!("{label} transport failed"), error))?; + if result.is_success() { + return Ok(result); + } + Err(clone_failure_error( + result.into_exec_error_with_redactor(label, |output| redact_auth_url(output, auth_url)), + CloneStep::Local, + has_app, + )) +} + +fn clone_failure_error(error: crate::Error, step: CloneStep, has_app: bool) -> crate::Error { + let message = match step { + CloneStep::Network if !has_app => { + "Git clone failed. If this is a private repository, configure a GitHub App with \ + `fabro install` and install it for your organization." + } + CloneStep::Network => "Failed to clone repository into the sandbox", + CloneStep::Local => "Failed to prepare the cloned repository in the sandbox", + }; + crate::Error::context(message, error) +} + +/// Point `origin` at the authenticated URL so pushes from the checkout +/// carry the clone token, and record that generation for refreshes. A +/// failure here is logged, not fatal: the checkout is complete, and the +/// first push will re-embed. +async fn embed_origin_credentials( + exec: &SandboxExec<'_>, + layout: &GitHubRepoLayout, + auth_url: Option<&DisplaySafeUrl>, + token: ResolvedToken, + credentials: &PushCredentialState, +) { + credentials.record_embedded(token).await; + let Some(auth_url) = auth_url else { + return; + }; + let command = format!( + "git -c maintenance.auto=0 remote set-url origin {}", + shell_quote(auth_url.as_raw_url().as_str()) + ); + match exec + .run( + &command, + Some(STEP_TIMEOUT), + Some(&layout.execution_directory), + None, + None, + ) + .await + { + Ok(result) if result.is_success() => {} + Ok(result) => { + let err = result + .into_exec_error_with_redactor("git remote set-url origin (post-clone)", |s| { + redact_auth_url(s, Some(auth_url)) + }); + tracing::warn!( + error = %err, + "Failed to set sandbox push credentials on origin; git push from this sandbox will fail" + ); + } + Err(err) => { + tracing::warn!( + error = %redact_auth_url(&crate::display_for_log(&err), Some(auth_url)), + "Failed to set sandbox push credentials on origin; git push from this sandbox will fail" + ); + } + } +} + +fn millis(duration: Duration) -> u64 { + u64::try_from(duration.as_millis()).unwrap_or(u64::MAX) +} diff --git a/lib/components/fabro-sandbox/src/clone_source.rs b/lib/components/fabro-sandbox/src/clone_source.rs index f9667f6bc..a8ea6b7d4 100644 --- a/lib/components/fabro-sandbox/src/clone_source.rs +++ b/lib/components/fabro-sandbox/src/clone_source.rs @@ -73,7 +73,6 @@ pub(crate) fn repo_symlink_command(layout: &GitHubRepoLayout) -> String { ) } -#[cfg(any(feature = "docker", test))] pub(crate) fn exact_repository_init_command(clone_url: &str, checkout_path: &str) -> String { format!( "{git} init -- {path} && git -C {path} remote add origin {origin}", @@ -159,7 +158,6 @@ pub(crate) fn tag_ref(tag: &str) -> String { /// /// The fetch names the revision directly rather than the branch, and /// `--no-tags` keeps unrelated tags from being pulled alongside it. -#[cfg(any(feature = "docker", test))] pub(crate) fn pinned_fetch_command( checkout_path: &str, fetch_source: &str, @@ -178,7 +176,6 @@ pub(crate) fn pinned_fetch_command( /// Leading-space ` --depth N` fragment for a Git command, or empty when /// `depth` is `None` to fetch full history. -#[cfg(any(feature = "docker", test))] pub(crate) fn depth_argument(depth: Option) -> String { depth.map_or_else(String::new, |depth| format!(" --depth {depth}")) } @@ -215,7 +212,6 @@ pub(crate) fn exact_head_revision_command(checkout_path: &str) -> String { /// Check out the admitted branch and print the resulting HEAD in one shell /// command; stdout is the `rev-parse HEAD` output for /// [`PinnedRevision::verify_head`]. -#[cfg(any(feature = "docker", test))] pub(crate) fn exact_checkout_verify_command( checkout_path: &str, branch: &str, @@ -230,7 +226,6 @@ pub(crate) fn exact_checkout_verify_command( /// The peeled commit behind whatever `git fetch` just wrote to `FETCH_HEAD`; /// a commit peels to itself, an annotated tag to the commit it points at. -#[cfg(any(feature = "docker", test))] pub(crate) const FETCH_HEAD_COMMIT: &str = "FETCH_HEAD^{commit}"; /// Validate that a `rev-parse HEAD` output is a single commit ID and return it diff --git a/lib/components/fabro-sandbox/src/details.rs b/lib/components/fabro-sandbox/src/details.rs index de54fa77e..2bbe8f45b 100644 --- a/lib/components/fabro-sandbox/src/details.rs +++ b/lib/components/fabro-sandbox/src/details.rs @@ -1,20 +1,20 @@ use std::collections::BTreeMap; use anyhow::Result; -#[cfg(any(feature = "docker", feature = "daytona"))] use chrono::{DateTime, Utc}; use fabro_types::{ BundledProvider, RunId, RunSandboxInstance, SandboxDetails, SandboxNetwork, SandboxResources, SandboxState, SandboxTimestamps, }; +use crate::docker; + /// Inspect the sandbox identified by `record` and return provider-neutral /// details for control-plane display. /// -/// Provider feature flags determine which branches resolve real data: /// - `local` always returns a minimal record describing the host. -/// - `docker` inspects the managed container through Bollard. -/// - `daytona` reconnects to the SDK sandbox. +/// - `docker` describes the managed container through the sandbox driver. +/// - `daytona` reconnects to the SDK sandbox (feature-gated). #[allow( unused_variables, reason = "Feature-gated providers consume some parameters only when enabled." @@ -27,8 +27,7 @@ pub async fn sandbox_details( ) -> Result { match record.provider.bundled() { Some(BundledProvider::Local) => Ok(local_details(record)), - #[cfg(feature = "docker")] - Some(BundledProvider::Docker) => docker::docker_details(record, run_id).await, + Some(BundledProvider::Docker) => docker_details(record, run_id).await, #[cfg(feature = "daytona")] Some(BundledProvider::Daytona) => daytona::daytona_details(record, daytona_api_key).await, _ => Err(anyhow::anyhow!( @@ -52,434 +51,128 @@ fn local_details(record: &RunSandboxInstance) -> SandboxDetails { } } -#[cfg(any(feature = "docker", feature = "daytona"))] +#[cfg(feature = "daytona")] fn parse_rfc3339_utc(value: &str) -> Option> { DateTime::parse_from_rfc3339(value) .ok() .map(|dt| dt.with_timezone(&Utc)) } -#[cfg(feature = "docker")] -pub(crate) mod docker { - use std::collections::BTreeMap; - - use anyhow::{Context, Result, anyhow}; - use bollard::Docker; - use bollard::container::InspectContainerOptions; - use bollard::models::{ContainerInspectResponse, ContainerStateStatusEnum, HostConfig}; - use fabro_types::{ - RunId, RunSandboxInstance, SandboxDetails, SandboxInfo, SandboxNetwork, - SandboxNetworkPolicy, SandboxProviderKind, SandboxResources, SandboxState, - SandboxTimestamps, - }; - - use super::parse_rfc3339_utc; - use crate::docker::WORKING_DIRECTORY; - - pub(super) async fn docker_details( - record: &RunSandboxInstance, - _run_id: Option, - ) -> Result { - let docker = - Docker::connect_with_local_defaults().context("Failed to connect to Docker daemon")?; - let runtime = &record.runtime; - let inspect = docker - .inspect_container(&runtime.id, None::) - .await - .map_err(|err| anyhow!("Failed to inspect Docker container '{}': {err}", runtime.id))?; - Ok(map_docker_inspect(&inspect, record)) +/// Projection of a sandbox-driver [`sandbox_driver::SandboxStatus`] into +/// fabro's inventory shape. The driver reports what a provider exposes +/// through its public facets; fields no facet carries (network policy) stay +/// unknown rather than being read from provider SDK types. +pub(crate) fn info_from_status( + kind: &fabro_types::SandboxProviderKind, + status: &sandbox_driver::SandboxStatus, +) -> fabro_types::SandboxInfo { + let fields = fields_from_status(status); + fabro_types::SandboxInfo { + provider: kind.clone(), + id: status.id.to_string(), + display_name: status.name.clone().filter(|name| !name.is_empty()), + state: fields.state, + native_state: fields.native_state, + image: status.source.clone(), + snapshot: None, + region: status.region.clone(), + web_url: status.web_url.clone(), + working_directory: None, + resources: fields.resources, + network: SandboxNetwork::unknown(), + labels: status.labels.clone(), + timestamps: fields.timestamps, } +} - pub(crate) fn docker_info_from_inspect(inspect: &ContainerInspectResponse) -> SandboxInfo { - let fields = docker_fields_from_inspect(inspect); - SandboxInfo { - provider: SandboxProviderKind::DOCKER, - id: fields.id, - display_name: fields.display_name, - state: fields.state, - native_state: fields.native_state, - image: fields.image, - snapshot: None, - region: None, - web_url: None, - working_directory: fields.working_directory, - resources: fields.resources, - network: fields.network, - labels: fields.labels, - timestamps: fields.timestamps, - } +pub(crate) fn details_from_status( + record: &RunSandboxInstance, + status: &sandbox_driver::SandboxStatus, +) -> SandboxDetails { + let fields = fields_from_status(status); + SandboxDetails { + sandbox: RunSandboxInstance { + image: status.source.clone().or_else(|| record.image.clone()), + ..record.clone() + }, + state: fields.state, + native_state: fields.native_state, + region: status.region.clone(), + web_url: status.web_url.clone(), + resources: fields.resources, + network: SandboxNetwork::unknown(), + labels: status.labels.clone(), + timestamps: fields.timestamps, } +} - pub(super) fn map_docker_inspect( - inspect: &ContainerInspectResponse, - record: &RunSandboxInstance, - ) -> SandboxDetails { - let fields = docker_fields_from_inspect(inspect); - let image = fields.image.clone().or_else(|| record.image.clone()); +struct StatusFields { + state: SandboxState, + native_state: Option, + resources: SandboxResources, + timestamps: SandboxTimestamps, +} - SandboxDetails { - sandbox: RunSandboxInstance { - image, - ..record.clone() - }, - state: fields.state, - native_state: fields.native_state, - region: None, - web_url: None, - resources: fields.resources, - network: fields.network, - labels: fields.labels, - timestamps: fields.timestamps, - } - } - - struct DockerFields { - id: String, - display_name: Option, - state: SandboxState, - native_state: Option, - image: Option, - working_directory: Option, - resources: SandboxResources, - network: SandboxNetwork, - labels: BTreeMap, - timestamps: SandboxTimestamps, - } - - fn docker_fields_from_inspect(inspect: &ContainerInspectResponse) -> DockerFields { - let status_enum = inspect - .state +fn fields_from_status(status: &sandbox_driver::SandboxStatus) -> StatusFields { + StatusFields { + state: normalize_driver_state(status.state), + native_state: Some(status.provider_state.clone()).filter(|value| !value.is_empty()), + resources: status + .resources .as_ref() - .and_then(|state| state.status.as_ref()) - .copied(); - let normalized_state = status_enum.map_or(SandboxState::Unknown, normalize_docker_state); - let native_state = status_enum - .map(|status| status.to_string()) - .filter(|value| !value.is_empty()); - - let host_config = inspect.host_config.as_ref(); - let resources = SandboxResources { - cpu_cores: host_config.and_then(docker_cpu_cores), - memory_bytes: host_config - .and_then(|host| host.memory) - .filter(|bytes| *bytes > 0) - .and_then(|bytes| u64::try_from(bytes).ok()), - disk_bytes: None, - }; - let network = docker_network(host_config); - - let labels: BTreeMap = inspect - .config - .as_ref() - .and_then(|config| config.labels.clone()) - .map(|map| map.into_iter().collect()) - .unwrap_or_default(); - - let image = inspect - .config - .as_ref() - .and_then(|config| config.image.clone()) - .or_else(|| inspect.image.clone()) - .filter(|value| !value.is_empty()); - let working_directory = inspect - .config - .as_ref() - .and_then(|config| config.working_dir.clone()) - .filter(|value| !value.is_empty()) - .or_else(|| Some(WORKING_DIRECTORY.to_string())); - - let id = inspect - .id - .clone() - .or_else(|| inspect.name.as_ref().map(|name| trim_container_name(name))) - .unwrap_or_default(); - let display_name = inspect - .name - .as_ref() - .map(|name| trim_container_name(name)) - .filter(|name| !name.is_empty()); - - let created_at = inspect.created.as_deref().and_then(parse_rfc3339_utc); - - DockerFields { - id, - display_name, - state: normalized_state, - native_state, - image, - working_directory, - resources, - network, - labels, - timestamps: SandboxTimestamps { - created_at, - last_activity_at: None, - }, - } + .map(|resources| SandboxResources { + cpu_cores: resources.cpu_cores.map(f64::from), + memory_bytes: resources.memory_mb.map(|mb| mb * 1024 * 1024), + disk_bytes: resources.disk_mb.map(|mb| mb * 1024 * 1024), + }) + .unwrap_or_default(), + timestamps: SandboxTimestamps { + created_at: status.created_at.map(DateTime::::from), + last_activity_at: status.updated_at.map(DateTime::::from), + }, } +} - fn trim_container_name(name: &str) -> String { - name.strip_prefix('/').unwrap_or(name).to_string() +pub(crate) fn normalize_driver_state(state: sandbox_driver::SandboxState) -> SandboxState { + use sandbox_driver::SandboxState as Driver; + match state { + Driver::Creating | Driver::Forking => SandboxState::Provisioning, + Driver::Starting | Driver::Resuming => SandboxState::Starting, + // A sandbox mid-snapshot keeps serving commands. + Driver::Running | Driver::Snapshotting => SandboxState::Running, + Driver::Stopping | Driver::Archiving => SandboxState::Stopping, + Driver::Stopped => SandboxState::Stopped, + Driver::Pausing | Driver::Paused => SandboxState::Paused, + Driver::Archived => SandboxState::Archived, + Driver::Restoring => SandboxState::Restoring, + Driver::Resizing => SandboxState::Resizing, + Driver::Deleting => SandboxState::Deleting, + Driver::Deleted => SandboxState::Deleted, + Driver::Error => SandboxState::Error, + _ => SandboxState::Unknown, } +} - fn docker_network(host_config: Option<&HostConfig>) -> SandboxNetwork { - match host_config.and_then(|host| host.network_mode.as_deref()) { - Some("none") => { - let blocked = SandboxNetworkPolicy::blocked(); - SandboxNetwork { - egress: blocked.clone(), - ingress: blocked, - } - } - _ => SandboxNetwork::unknown(), - } - } - - pub(super) fn docker_cpu_cores(host_config: &HostConfig) -> Option { - let quota = host_config.cpu_quota?; - let period = host_config.cpu_period?; - if quota <= 0 || period <= 0 { - return None; - } - #[allow( - clippy::cast_precision_loss, - reason = "CPU quota/period are bounded well within f64 mantissa precision." - )] - let cores = (quota as f64) / (period as f64); - Some(cores) - } - - pub(super) fn normalize_docker_state(status: ContainerStateStatusEnum) -> SandboxState { - match status { - ContainerStateStatusEnum::EMPTY => SandboxState::Unknown, - ContainerStateStatusEnum::CREATED => SandboxState::Provisioning, - ContainerStateStatusEnum::RUNNING => SandboxState::Running, - ContainerStateStatusEnum::PAUSED => SandboxState::Paused, - ContainerStateStatusEnum::RESTARTING => SandboxState::Starting, - ContainerStateStatusEnum::REMOVING => SandboxState::Deleting, - ContainerStateStatusEnum::EXITED => SandboxState::Stopped, - ContainerStateStatusEnum::DEAD => SandboxState::Error, - } - } - - #[cfg(test)] - mod tests { - use bollard::models::HostConfig; - use fabro_types::{ - RunSandboxInstance, RunSandboxRuntime, SandboxNetwork, SandboxNetworkPolicy, - SandboxProviderKind, - }; - - use super::*; - - fn record() -> RunSandboxInstance { - RunSandboxInstance { - provider: SandboxProviderKind::DOCKER, - image: None, - snapshot: None, - runtime: RunSandboxRuntime { - id: "container-abc123".to_string(), - working_directory: "/workspace".to_string(), - repo_cloned: Some(true), - clone_origin_url: None, - clone_branch: None, - workspace_root: None, - repos_root: None, - primary_repo_path: None, - primary_repo_link: None, - }, - } - } - - #[test] - fn cpu_cores_divides_quota_by_period() { - let host = HostConfig { - cpu_quota: Some(200_000), - cpu_period: Some(100_000), - ..Default::default() - }; - assert_eq!(docker_cpu_cores(&host), Some(2.0)); - } - - #[test] - fn cpu_cores_returns_none_when_quota_missing() { - let host = HostConfig { - cpu_quota: None, - cpu_period: Some(100_000), - ..Default::default() - }; - assert_eq!(docker_cpu_cores(&host), None); - } - - #[test] - fn cpu_cores_returns_none_when_period_zero() { - let host = HostConfig { - cpu_quota: Some(200_000), - cpu_period: Some(0), - ..Default::default() - }; - assert_eq!(docker_cpu_cores(&host), None); - } - - #[test] - fn memory_bytes_zero_is_unset() { - let inspect = ContainerInspectResponse { - host_config: Some(HostConfig { - memory: Some(0), - ..Default::default() - }), - ..Default::default() - }; - let details = map_docker_inspect(&inspect, &record()); - assert_eq!(details.resources.memory_bytes, None); - } - - #[test] - fn memory_bytes_present_is_carried_through() { - let inspect = ContainerInspectResponse { - host_config: Some(HostConfig { - memory: Some(2 * 1024 * 1024 * 1024), - ..Default::default() - }), - ..Default::default() - }; - let details = map_docker_inspect(&inspect, &record()); - assert_eq!(details.resources.memory_bytes, Some(2_147_483_648)); - } - - #[test] - fn network_mode_none_blocks_ingress_and_egress() { - let inspect = ContainerInspectResponse { - host_config: Some(HostConfig { - network_mode: Some("none".to_string()), - ..Default::default() - }), - ..Default::default() - }; - let details = map_docker_inspect(&inspect, &record()); - assert_eq!(details.network.egress, SandboxNetworkPolicy::blocked()); - assert_eq!(details.network.ingress, SandboxNetworkPolicy::blocked()); - } - - #[test] - fn non_none_network_mode_is_unknown() { - let inspect = ContainerInspectResponse { - host_config: Some(HostConfig { - network_mode: Some("bridge".to_string()), - ..Default::default() - }), - ..Default::default() - }; - let details = map_docker_inspect(&inspect, &record()); - assert_eq!(details.network, SandboxNetwork::unknown()); - } - - #[test] - fn record_identity_is_carried_through() { - let inspect = ContainerInspectResponse { - name: Some("/fabro-run-abc".to_string()), - ..Default::default() - }; - let details = map_docker_inspect(&inspect, &record()); - let runtime = details.sandbox.runtime; - assert_eq!(runtime.id, "container-abc123"); - assert_eq!(runtime.working_directory, "/workspace"); - } - - #[test] - fn inventory_identity_uses_native_id_and_display_name() { - let inspect = ContainerInspectResponse { - id: Some("container-abc123".to_string()), - name: Some("/fabro-run-abc".to_string()), - ..Default::default() - }; - let info = docker_info_from_inspect(&inspect); - assert_eq!(info.id, "container-abc123"); - assert_eq!(info.display_name.as_deref(), Some("fabro-run-abc")); - } - - #[test] - fn inventory_working_directory_defaults_to_fabro_workspace() { - let inspect = ContainerInspectResponse::default(); - let info = docker_info_from_inspect(&inspect); - assert_eq!(info.working_directory.as_deref(), Some("/workspace")); - } - - #[test] - fn empty_status_is_unknown() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::EMPTY), - SandboxState::Unknown - ); - } - - #[test] - fn created_status_is_provisioning() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::CREATED), - SandboxState::Provisioning - ); - } - - #[test] - fn running_status_is_running() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::RUNNING), - SandboxState::Running - ); - } - - #[test] - fn paused_status_is_paused() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::PAUSED), - SandboxState::Paused - ); - } - - #[test] - fn restarting_status_is_starting() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::RESTARTING), - SandboxState::Starting - ); - } - - #[test] - fn removing_status_is_deleting() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::REMOVING), - SandboxState::Deleting - ); - } - - #[test] - fn exited_status_is_stopped() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::EXITED), - SandboxState::Stopped - ); - } - - #[test] - fn dead_status_is_error() { - assert_eq!( - normalize_docker_state(ContainerStateStatusEnum::DEAD), - SandboxState::Error - ); - } - - #[test] - fn parse_timestamp_accepts_rfc3339() { - let parsed = parse_rfc3339_utc("2026-05-09T12:00:00Z"); - assert!(parsed.is_some()); - } - - #[test] - fn parse_timestamp_rejects_garbage() { - assert!(parse_rfc3339_utc("not a date").is_none()); - } - } +async fn docker_details( + record: &RunSandboxInstance, + run_id: Option, +) -> Result { + let runtime = &record.runtime; + let sandbox = docker::attach_docker( + &runtime.id, + runtime.repo_cloned.unwrap_or(false), + runtime.working_directory.clone(), + runtime.clone_origin_url.clone(), + run_id, + ) + .await?; + let status = sandbox.handle()?.describe().await.map_err(|err| { + anyhow::anyhow!( + "Failed to describe Docker container '{}': {err}", + runtime.id + ) + })?; + Ok(details_from_status(record, &status)) } #[cfg(feature = "daytona")] @@ -823,9 +516,86 @@ pub(crate) mod daytona { #[cfg(test)] mod tests { use fabro_types::SandboxProviderKind; + use sandbox_driver::SandboxId; use super::*; + #[test] + fn driver_states_map_onto_fabro_states() { + use sandbox_driver::SandboxState as Driver; + for (driver, fabro) in [ + (Driver::Creating, SandboxState::Provisioning), + (Driver::Starting, SandboxState::Starting), + (Driver::Running, SandboxState::Running), + (Driver::Snapshotting, SandboxState::Running), + (Driver::Stopping, SandboxState::Stopping), + (Driver::Stopped, SandboxState::Stopped), + (Driver::Paused, SandboxState::Paused), + (Driver::Archived, SandboxState::Archived), + (Driver::Deleting, SandboxState::Deleting), + (Driver::Deleted, SandboxState::Deleted), + (Driver::Error, SandboxState::Error), + (Driver::Unknown, SandboxState::Unknown), + ] { + assert_eq!(normalize_driver_state(driver), fabro, "{driver:?}"); + } + } + + #[test] + fn status_projection_carries_identity_source_and_labels() { + let mut status = sandbox_driver::SandboxStatus::new( + SandboxId::try_new("container-abc123").unwrap(), + sandbox_driver::SandboxState::Running, + ); + status.name = Some("fabro-run-abc".to_string()); + status.provider_state = "running".to_string(); + status.source = Some("buildpack-deps:noble".to_string()); + status + .labels + .insert("sh.fabro.managed".to_string(), "true".to_string()); + let mut resources = sandbox_driver::Resources::default(); + resources.cpu_cores = Some(2); + resources.memory_mb = Some(2048); + status.resources = Some(resources); + + let info = info_from_status(&SandboxProviderKind::DOCKER, &status); + assert_eq!(info.id, "container-abc123"); + assert_eq!(info.display_name.as_deref(), Some("fabro-run-abc")); + assert_eq!(info.state, SandboxState::Running); + assert_eq!(info.native_state.as_deref(), Some("running")); + assert_eq!(info.image.as_deref(), Some("buildpack-deps:noble")); + assert_eq!(info.resources.cpu_cores, Some(2.0)); + assert_eq!(info.resources.memory_bytes, Some(2_147_483_648)); + assert_eq!( + info.labels.get("sh.fabro.managed").map(String::as_str), + Some("true") + ); + + let record = RunSandboxInstance { + provider: SandboxProviderKind::DOCKER, + image: None, + snapshot: None, + runtime: fabro_types::RunSandboxRuntime { + id: "container-abc123".to_string(), + working_directory: "/workspace".to_string(), + repo_cloned: Some(true), + clone_origin_url: None, + clone_branch: None, + workspace_root: None, + repos_root: None, + primary_repo_path: None, + primary_repo_link: None, + }, + }; + let details = details_from_status(&record, &status); + assert_eq!( + details.sandbox.image.as_deref(), + Some("buildpack-deps:noble") + ); + assert_eq!(details.sandbox.runtime.id, "container-abc123"); + assert_eq!(details.network, SandboxNetwork::unknown()); + } + #[test] fn local_details_returns_running_with_no_metadata() { let record = RunSandboxInstance { diff --git a/lib/components/fabro-sandbox/src/docker.rs b/lib/components/fabro-sandbox/src/docker.rs index c728081ea..a177f54bd 100644 --- a/lib/components/fabro-sandbox/src/docker.rs +++ b/lib/components/fabro-sandbox/src/docker.rs @@ -1,123 +1,39 @@ -use std::collections::HashMap; -use std::fmt::Write as _; -use std::io::Cursor; -use std::sync::Arc; -use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; -use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +//! The `docker` provider kind: fabro's environment mapping onto the +//! sandbox-driver Docker provider. +//! +//! Fabro decides the image, resources, network policy, environment, labels, +//! and workspace layout; the driver creates and drives the container. The +//! container's working directory is [`WORKING_DIRECTORY`]; a cloned +//! repository checks out under [`REPOS_ROOT`] and is linked into the +//! workspace, so the run works in `/workspace/`. + +use std::collections::BTreeMap; -use async_trait::async_trait; -use bollard::Docker; -use bollard::container::{ - Config, CreateContainerOptions, DownloadFromContainerOptions, InspectContainerOptions, - LogOutput, RemoveContainerOptions, StartContainerOptions, StopContainerOptions, - UploadToContainerOptions, -}; -use bollard::errors::Error as DockerError; -use bollard::exec::{CreateExecOptions, StartExecOptions, StartExecResults}; -use bollard::image::CreateImageOptions; -use bollard::models::{ContainerInspectResponse, HostConfig}; use fabro_github::GitHubCredentials; -use fabro_github::token_source::InstallationTokenSource; use fabro_types::settings::run::RunCloneSettings; -use fabro_types::{CommandOutputStream, CommandTermination, RunId, SandboxProviderKind}; -use fabro_util::time::elapsed_ms; -use futures::StreamExt; -use tokio::io::{AsyncWriteExt, duplex}; -use tokio::sync::{Mutex as TokioMutex, Notify, OnceCell}; -use tokio::{fs, time}; -use tokio_util::sync::CancellationToken; - -use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason}; -use crate::git_retry::{self, CredentialContext}; -use crate::managed_labels::{self, MANAGED_LABEL, RUN_ID_LABEL}; -use crate::push_credentials::{self, PushCredentialState}; -use crate::redact::redact_auth_url; -use crate::sandbox::{ - self, BASH_ENV_VAR, BASH_PROBE_SCRIPT, BASH_PROBE_TIMEOUT_MS, OutputCaptureBuffer, REMOTE_BASH, - REMOTE_WALK_TIMEOUT_MS, RefreshOutcome, StdioProcessControl, optional_timeout, resolve_path, - validate_bash_probe, write_process_stdin, -}; -use crate::{ - CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult, - ExecStreamingRequest, ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, - SandboxEventCallback, SandboxFile, StderrCollector, StdioProcess, StdioProcessHandle, - StdioProcessTermination, WalkOptions, format_lines_numbered, shell_quote, +use fabro_types::settings::server::ServerSandboxProviderSettings; +use fabro_types::{RunId, SandboxProviderKind}; +use sandbox_driver::{ + HealthStatus, NetworkPolicy, Resources, SandboxId, SandboxSource, SandboxSpec as DriverSpec, }; +use sandbox_driver_docker_config::DockerProviderConfig; -const DOCKER_BASH_REQUIREMENT: &str = "Docker sandboxes require /bin/bash for every command, with no `sh` fallback; use an \ - image with bash and git, such as buildpack-deps:noble."; +use crate::driver::{ProviderConnectOptions, connect_provider}; +use crate::driver_sandbox::{DriverSandbox, RepoWorkspace, WorkspaceLayout}; +use crate::managed_labels::{self, MANAGED_LABEL, MANAGED_LABEL_VALUE, RUN_ID_LABEL}; -pub(crate) const WORKING_DIRECTORY: &str = "/workspace"; -pub(crate) const REPOS_ROOT: &str = "/repos"; -// Beneath the system tmp dir so any container user can create it; the -// trailing `runtime` component is load-bearing — materialized blobs at -// `runtime/blobs/{hash}.json` are recognized as managed blob references and -// normalized back to `blob://` in durable context. -pub(crate) const RUNTIME_DIRECTORY: &str = "/tmp/fabro/runtime"; +pub const WORKING_DIRECTORY: &str = "/workspace"; +pub const REPOS_ROOT: &str = "/repos"; const DEFAULT_GIT_CLONE_DEPTH: usize = RunCloneSettings::DEFAULT_DEPTH.unsigned_abs() as usize; -const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(5); -#[cfg(test)] -const EXEC_STOP_POLL_SLEEP_SECONDS: &str = "0.005"; -#[cfg(not(test))] -const EXEC_STOP_POLL_SLEEP_SECONDS: &str = "0.1"; -#[cfg(test)] -const EXEC_TERM_GRACE_SECONDS: &str = "0.02"; -#[cfg(not(test))] -const EXEC_TERM_GRACE_SECONDS: &str = "0.2"; - -/// Whether a failing git step talked to the remote. Local steps cannot fail on -/// credentials, so they must not suggest reconfiguring the GitHub App. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum CloneStep { - Network, - Local, -} - -struct DockerCloneFailure { - error: crate::Error, - retry_reason: Option, -} - -fn env_entry_name(entry: &str) -> &str { - entry.split_once('=').map_or(entry, |(name, _)| name) -} - -/// Remove caller/image startup-file injection and explicitly override any -/// inherited image value for Docker-created processes. -fn clean_bash_env_entries(entries: impl IntoIterator) -> Vec { - let mut clean: Vec = entries - .into_iter() - .filter(|entry| env_entry_name(entry) != BASH_ENV_VAR) - .collect(); - clean.push(format!("{BASH_ENV_VAR}=")); - clean -} - -fn docker_bash_exec_env(env_vars: Option<&HashMap>) -> Vec { - let entries = env_vars.map_or_else(Vec::new, |vars| { - vars.iter() - .map(|(key, value)| format!("{key}={value}")) - .collect() - }); - clean_bash_env_entries(entries) -} - -static EXEC_CONTROL_COUNTER: AtomicU64 = AtomicU64::new(1); - -pub fn docker_access_command(container_id: &str, working_directory: &str) -> String { - let shell = format!("cd {} && exec sh -l", shell_quote(working_directory)); - format!( - "docker exec -it {} sh -lc {}", - shell_quote(container_id), - shell_quote(&shell) - ) -} +/// Docker's default CFS period; a whole core is one period's worth of quota. +const CPU_PERIOD_MICROS: i64 = 100_000; +/// Options fabro derives from an environment for a Docker sandbox. #[derive(Clone, Debug, PartialEq)] pub struct DockerSandboxOptions { /// Docker image to use. pub image: String, - /// Docker network mode. Default: `Some("bridge")`. + /// Docker network mode. Default: `Some("bridge")`; `Some("none")` blocks. pub network_mode: Option, /// Memory limit in bytes. `None` = unlimited. pub memory_limit: Option, @@ -149,1507 +65,166 @@ impl Default for DockerSandboxOptions { } } -pub struct DockerSandbox { - docker: Docker, - config: DockerSandboxOptions, - push_credentials: PushCredentialState, - run_id: Option, - clone_origin_url: Option, - clone_branch: Option, - clone_tag: Option, - clone_commit_sha: Option, - container_id: OnceCell, - repo_cloned: OnceCell, - working_directory: OnceCell, - origin_url: OnceCell, - cached_platform: std::sync::OnceLock, - cached_os_version: std::sync::OnceLock, - rg_available: OnceCell, - event_callback: Option, -} - -enum EnsureImageOutcome { - Skipped, - AlreadyLocal, - Pulled, -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq, strum::Display)] -#[strum(serialize_all = "lowercase")] -enum ContainerStartAction { - Start, - Unpause, -} - -impl DockerSandbox { - pub fn new( - config: DockerSandboxOptions, - github_app: Option<&GitHubCredentials>, - run_id: Option, - clone_origin_url: Option, - clone_branch: Option, - clone_tag: Option, - clone_commit_sha: Option, - ) -> crate::Result { - if clone_tag.is_some() || clone_commit_sha.is_some() { - clone_source::decide_clone( - config.skip_clone, - clone_origin_url.as_deref(), - clone_branch.as_deref(), - clone_tag.as_deref(), - clone_commit_sha.as_deref(), - )?; - } - let docker = Docker::connect_with_local_defaults().map_err(crate::Error::docker_connect)?; - Self::with_docker_client( - docker, - config, - github_app, - run_id, - clone_origin_url, - clone_branch, - clone_tag, - clone_commit_sha, - ) - } - - fn with_docker_client( - docker: Docker, - config: DockerSandboxOptions, - github_app: Option<&GitHubCredentials>, - run_id: Option, - clone_origin_url: Option, - clone_branch: Option, - clone_tag: Option, - clone_commit_sha: Option, - ) -> crate::Result { - let push_credentials = PushCredentialState::new(push_credentials::build_token_source( - github_app, - clone_origin_url.as_deref(), - )?); - Ok(Self { - docker, - config, - push_credentials, - run_id, - clone_origin_url, - clone_branch, - clone_tag, - clone_commit_sha, - container_id: OnceCell::new(), - repo_cloned: OnceCell::new(), - working_directory: OnceCell::new(), - origin_url: OnceCell::new(), - cached_platform: std::sync::OnceLock::new(), - cached_os_version: std::sync::OnceLock::new(), - rg_available: OnceCell::const_new(), - event_callback: None, - }) - } - - pub async fn reconnect( - container_id: &str, - repo_cloned: bool, - working_directory: String, - clone_origin_url: Option, - clone_branch: Option, - run_id: Option, - ) -> crate::Result { - let sandbox = Self::new( - DockerSandboxOptions::default(), - None, - run_id, - clone_origin_url.clone(), - clone_branch, - None, - None, - )?; - sandbox.validate_managed_container(container_id).await?; - sandbox - .container_id - .set(container_id.to_string()) - .map_err(|_| "Container already initialized".to_string())?; - sandbox - .repo_cloned - .set(repo_cloned) - .map_err(|_| "Clone state already initialized".to_string())?; - sandbox - .working_directory - .set(working_directory) - .map_err(|_| "Working directory already initialized".to_string())?; - if repo_cloned { - if let Some(origin) = clone_origin_url { - let _ = sandbox.origin_url.set(origin); - } - } - Ok(sandbox) - } - - pub fn set_event_callback(&mut self, cb: SandboxEventCallback) { - self.event_callback = Some(cb); - } - - fn emit(&self, event: SandboxEvent) { - event.trace(); - if let Some(ref cb) = self.event_callback { - cb(event); - } - } - - fn container_id(&self) -> crate::Result<&str> { - self.container_id.get().map(String::as_str).ok_or_else(|| { - crate::Error::message("Container not initialized — call initialize() first") - }) - } - - pub(crate) fn container_identifier(&self) -> crate::Result<&str> { - self.container_id() - } - - pub(crate) fn docker_client(&self) -> Docker { - self.docker.clone() - } - - fn resolve_container_path(&self, path: &str) -> String { - resolve_path(path, self.working_directory()) - } - - async fn download_file_bytes(&self, remote_path: &str) -> crate::Result> { - #[expect( - clippy::disallowed_types, - reason = "tar entries are synchronous in-memory readers; bytes are collected before any await" - )] - use std::io::Read as _; - - let container_id = self.container_id()?; - let container_path = self.resolve_container_path(remote_path); - let opts = DownloadFromContainerOptions { - path: container_path.clone(), - }; - let mut stream = self - .docker - .download_from_container(container_id, Some(opts)); - let mut archive_bytes = Vec::new(); - while let Some(chunk) = stream.next().await { - let chunk = chunk.map_err(|e| { - crate::Error::context( - format!("Failed to download {container_path} from container"), - e, - ) - })?; - archive_bytes.extend_from_slice(&chunk); - } - - let mut archive = tar::Archive::new(Cursor::new(archive_bytes)); - let entries = archive.entries().map_err(|e| { - crate::Error::context( - format!("Failed to read Docker archive for {container_path}"), - e, - ) - })?; - for entry in entries { - let mut entry = entry.map_err(|e| { - crate::Error::context( - format!("Failed to read Docker archive entry for {container_path}"), - e, - ) - })?; - if !entry.header().entry_type().is_file() { - continue; - } - let mut bytes = Vec::new(); - entry.read_to_end(&mut bytes).map_err(|e| { - crate::Error::context( - format!("Failed to read Docker archive file for {container_path}"), - e, - ) - })?; - return Ok(bytes); - } - - Err(crate::Error::message(format!( - "Docker archive for {container_path} did not contain a file" - ))) - } - - fn repo_cloned(&self) -> bool { - self.repo_cloned.get().copied().unwrap_or(false) - } - - fn set_working_directory(&self, working_directory: impl Into) -> crate::Result<()> { - self.working_directory - .set(working_directory.into()) - .map_err(|_| crate::Error::message("Docker working directory already initialized")) - } - - async fn docker_exec( - &self, - cmd: Vec, - working_dir: Option<&str>, - env: Option>, - ) -> crate::Result<(String, String, i32)> { - let container_id = self.container_id()?; - - let exec_opts = CreateExecOptions { - cmd: Some(cmd), - attach_stdout: Some(true), - attach_stderr: Some(true), - working_dir: working_dir.map(ToString::to_string), - env: env.map(|e| e.into_iter().collect()), - ..Default::default() - }; - - let (exec_id, start_result) = create_and_start_exec( - &self.docker, - container_id, - exec_opts, - None, - "Failed to create exec", - "Failed to start exec", - ) - .await?; - - let mut stdout = String::new(); - let mut stderr = String::new(); - - if let StartExecResults::Attached { mut output, .. } = start_result { - while let Some(chunk) = output.next().await { - match chunk { - Ok(LogOutput::StdOut { message }) => { - stdout.push_str(&String::from_utf8_lossy(&message)); - } - Ok(LogOutput::StdErr { message }) => { - stderr.push_str(&String::from_utf8_lossy(&message)); - } - Ok(_) => {} - Err(e) => { - return Err(crate::Error::context("Error reading exec output", e)); - } - } - } - } - - let inspect = self - .docker - .inspect_exec(&exec_id) - .await - .map_err(|e| crate::Error::context("Failed to inspect exec", e))?; - - let exit_code = inspect - .exit_code - .and_then(|code| i32::try_from(code).ok()) - .unwrap_or(-1); - Ok((stdout, stderr, exit_code)) - } - - async fn docker_exec_streaming( - docker: Docker, - container_id: String, - cmd: Vec, - working_dir: Option, - env: Option>, - stdin: Option>, - output_callback: Option, - stream_output_bytes_cap: Option, - ) -> crate::Result<(OutputCaptureBuffer, OutputCaptureBuffer, i32)> { - let exec_opts = CreateExecOptions { - cmd: Some(cmd), - attach_stdin: Some(stdin.is_some()), - attach_stdout: Some(true), - attach_stderr: Some(true), - tty: Some(false), - working_dir, - env: env.map(|e| e.into_iter().collect()), - ..Default::default() - }; - - let (exec_id, start_result) = create_and_start_exec( - &docker, - &container_id, - exec_opts, - None, - "Failed to create exec", - "Failed to start exec", - ) - .await?; - - let mut stdout = OutputCaptureBuffer::new(stream_output_bytes_cap); - let mut stderr = OutputCaptureBuffer::new(stream_output_bytes_cap); - - let StartExecResults::Attached { mut output, input } = start_result else { - return Err(crate::Error::message( - "Docker started streaming command without attached standard I/O", - )); - }; - let write_stdin = async move { - if let Some(stdin) = stdin { - write_process_stdin(input, &stdin).await?; - } - crate::Result::Ok(()) - }; - let read_output = async { - while let Some(chunk) = output.next().await { - match chunk { - Ok(LogOutput::StdOut { message }) => { - stdout.push(&message); - if let Some(output_callback) = output_callback.as_ref() { - output_callback(CommandOutputStream::Stdout, message.to_vec()).await?; - } - } - Ok(LogOutput::StdErr { message }) => { - stderr.push(&message); - if let Some(output_callback) = output_callback.as_ref() { - output_callback(CommandOutputStream::Stderr, message.to_vec()).await?; - } - } - Ok(_) => {} - Err(e) => { - return Err(crate::Error::context("Error reading exec output", e)); - } - } - } - crate::Result::Ok(()) - }; - tokio::try_join!(write_stdin, read_output)?; - - let inspect = docker - .inspect_exec(&exec_id) - .await - .map_err(|e| crate::Error::context("Failed to inspect exec", e))?; - - let exit_code = inspect - .exit_code - .and_then(|code| i32::try_from(code).ok()) - .unwrap_or(-1); - Ok((stdout, stderr, exit_code)) - } - - async fn docker_exec_shell( - &self, - command: &str, - timeout_ms: u64, - working_dir: Option<&str>, - env_vars: Option<&HashMap>, - cancel_token: Option, - ) -> crate::Result { - let start = Instant::now(); - let effective_dir = working_dir - .unwrap_or_else(|| self.working_directory()) - .to_string(); - let env = Some(docker_bash_exec_env(env_vars)); - let cmd = vec![ - REMOTE_BASH.to_string(), - "-c".to_string(), - command.to_string(), - ]; - - let timeout_duration = std::time::Duration::from_millis(timeout_ms); - let token = cancel_token.unwrap_or_default(); - - tokio::select! { - result = self.docker_exec(cmd, Some(&effective_dir), env) => { - let (stdout, stderr, exit_code) = result?; - let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); - Ok(ExecResult { - stdout, - stderr, - exit_code: Some(exit_code), - termination: CommandTermination::Exited, - duration_ms, - }) - } - () = time::sleep(timeout_duration) => { - let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); - Ok(ExecResult { - stdout: String::new(), - stderr: "Command timed out".to_string(), - exit_code: None, - termination: CommandTermination::TimedOut, - duration_ms, - }) - } - () = token.cancelled() => { - let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); - Ok(ExecResult { - stdout: String::new(), - stderr: "Command cancelled".to_string(), - exit_code: None, - termination: CommandTermination::Cancelled, - duration_ms, - }) - } - } - } - - async fn docker_exec_shell_streaming( - &self, - request: ExecStreamingRequest<'_>, - ) -> crate::Result { - let ExecStreamingRequest { - command, - timeout_ms, - working_dir, - env_vars, - cancel_token, - stdin, - output_callback, - stream_output_bytes_cap, - } = request; - let start = Instant::now(); - let effective_dir = working_dir - .unwrap_or_else(|| self.working_directory()) - .to_string(); - let env = Some(docker_bash_exec_env(env_vars)); - let (stop_file, pid_file) = docker_exec_control_paths(); - let controlled_command = docker_controlled_shell_command(command, &stop_file, &pid_file); - let cmd = vec![ - REMOTE_BASH.to_string(), - "-c".to_string(), - controlled_command, - ]; - - let timeout_future = optional_timeout(timeout_ms); - tokio::pin!(timeout_future); - let token = cancel_token.unwrap_or_default(); - - let container_id = self.container_id()?.to_string(); - let mut output_task = tokio::spawn(Self::docker_exec_streaming( - self.docker.clone(), - container_id, - cmd, - Some(effective_dir.clone()), - env, - stdin, - output_callback, - stream_output_bytes_cap, - )); - - let mut termination = CommandTermination::Exited; - let output = tokio::select! { - joined = &mut output_task => { - joined - .map_err(|e| crate::Error::context("Docker exec stream task failed", e))?? - } - () = &mut timeout_future => { - termination = CommandTermination::TimedOut; - self.request_docker_exec_stop(&stop_file).await?; - output_task - .await - .map_err(|e| crate::Error::context("Docker exec stream task failed", e))?? - } - () = token.cancelled() => { - termination = CommandTermination::Cancelled; - self.request_docker_exec_stop(&stop_file).await?; - output_task - .await - .map_err(|e| crate::Error::context("Docker exec stream task failed", e))?? - } - }; - - let (stdout, stderr, exit_code) = output; - let (stdout, stdout_capture) = stdout.into_parts(); - let (stderr, stderr_capture) = stderr.into_parts(); - let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); - Ok(ExecStreamingResult { - result: ExecResult { - stdout: String::from_utf8_lossy(&stdout).into_owned(), - stderr: String::from_utf8_lossy(&stderr).into_owned(), - exit_code: (termination == CommandTermination::Exited).then_some(exit_code), - termination, - duration_ms, - }, - streams_separated: true, - live_streaming: true, - stdout_capture, - stderr_capture, - }) - } - - async fn request_docker_exec_stop(&self, stop_file: &str) -> crate::Result<()> { - request_docker_exec_stop_with(&self.docker, self.container_id()?, stop_file).await - } - - async fn ensure_image(&self) -> crate::Result { - if !self.config.auto_pull { - return Ok(EnsureImageOutcome::Skipped); - } - - match self.docker.inspect_image(&self.config.image).await { - Ok(_) => return Ok(EnsureImageOutcome::AlreadyLocal), - Err(e) if docker_not_found(&e) => {} - Err(e) => { - return Err(crate::Error::docker_image_inspect( - self.config.image.clone(), - e, - )); - } - } - - let (repo, tag) = if let Some((r, t)) = self.config.image.rsplit_once(':') { - (r.to_string(), t.to_string()) - } else { - (self.config.image.clone(), "latest".to_string()) - }; - - let opts = CreateImageOptions { - from_image: repo, - tag, - ..Default::default() - }; - - self.emit(SandboxEvent::SnapshotPulling { - name: self.config.image.clone(), - }); - let mut stream = self.docker.create_image(Some(opts), None, None); - while let Some(result) = stream.next().await { - result.map_err(|e| crate::Error::docker_image_pull(self.config.image.clone(), e))?; - } - - Ok(EnsureImageOutcome::Pulled) - } - - async fn create_workspace(&self) -> crate::Result<()> { - let result = self - .docker_exec_shell( - &format!("mkdir -p {}", shell_quote(WORKING_DIRECTORY)), - 10_000, - Some("/"), - None, - None, - ) - .await?; - if !result.is_success() { - return Err(crate::Error::message(format!( - "Failed to create Docker workspace (exit {}): {}", - result.display_exit_code(), - result.stderr - ))); - } - self.set_working_directory(WORKING_DIRECTORY)?; - Ok(()) - } - - /// Create the run-scoped Fabro runtime directory outside the repository - /// checkout. The umask keeps every created level owner-private. - async fn create_runtime_directory(&self) -> crate::Result<()> { - let result = self - .docker_exec_shell( - &format!("umask 077 && mkdir -p {}", shell_quote(RUNTIME_DIRECTORY)), - 10_000, - Some("/"), - None, - None, - ) - .await?; - if !result.is_success() { - return Err(crate::Error::message(format!( - "Failed to create Docker runtime directory (exit {}): {}", - result.display_exit_code(), - result.stderr - ))); - } - Ok(()) - } - - /// Verify the container evaluates commands as non-login Bash. - /// - /// Shared by fresh initialization and by `start` after a reconnect, so a - /// resumed container cannot pass startup and then fail on its first - /// command. - async fn probe_bash(&self, working_dir: Option<&str>) -> crate::Result<()> { - let result = self - .docker_exec_shell( - BASH_PROBE_SCRIPT, - BASH_PROBE_TIMEOUT_MS, - Some(working_dir.unwrap_or("/")), - None, - None, - ) - .await - .map_err(|err| crate::Error::context(DOCKER_BASH_REQUIREMENT, err))?; - validate_bash_probe(result, DOCKER_BASH_REQUIREMENT) - } - - async fn verify_git_available(&self) -> crate::Result<()> { - let result = self - .docker_exec_shell("git --version", 10_000, Some("/"), None, None) - .await?; - if !result.is_success() { - return Err(crate::Error::message(format!( - "Docker image '{}' must include git for repository clone and git lifecycle operations. Use an image with bash and git, such as buildpack-deps:noble.", - self.config.image - ))); - } - Ok(()) - } - - /// Preserve a failed git step result while masking the auth URL. - fn clone_failure_error( - &self, - result: ExecResult, - label: &'static str, - auth_url: Option<&fabro_redact::DisplaySafeUrl>, - step: CloneStep, - ) -> crate::Error { - let error = - result.into_exec_error_with_redactor(label, |output| redact_auth_url(output, auth_url)); - let message = match step { - CloneStep::Network if self.push_credentials.source().is_none() => { - "Git clone failed. If this is a private repository, configure a GitHub App with \ - `fabro install` and install it for your organization." - } - CloneStep::Network => "Failed to clone repository into Docker sandbox", - CloneStep::Local => "Failed to prepare the cloned repository in the Docker sandbox", - }; - crate::Error::context(message, error) - } - - fn report_clone_failure(&self, origin_url: &str, err: crate::Error) -> crate::Error { - self.emit(SandboxEvent::GitCloneFailed { - url: origin_url.to_string(), - error: err.to_string(), - causes: err.causes(), - }); - err - } - - /// Run a local (non-network) step of the exact checkout under the shared - /// clone deadline. - /// - /// Materializing a large working tree takes far longer than the short fixed - /// timeout used for trivial container commands, so these steps get the same - /// budget the branch clone path gives its fetch and checkout. - async fn run_exact_local_git_command( - &self, - command: &str, - label: &'static str, - clone_deadline: time::Instant, - auth_url: Option<&fabro_redact::DisplaySafeUrl>, - ) -> crate::Result { - let remaining = clone_deadline.saturating_duration_since(time::Instant::now()); - let timeout_ms = u64::try_from(remaining.as_millis()).unwrap_or(u64::MAX); - if timeout_ms == 0 { - return Err(crate::Error::message(format!( - "{label} deadline expired before the step could run" - ))); - } - let result = self - .docker_exec_shell(command, timeout_ms, Some("/"), None, None) - .await - .map_err(|error| crate::Error::context(format!("{label} transport failed"), error))?; - if result.is_success() { - Ok(result) - } else { - Err(self.clone_failure_error(result, label, auth_url, CloneStep::Local)) - } - } - - /// Run a network git command inside the container with clone retry - /// semantics under the shared clone deadline. - async fn retry_git_transfer( - &self, - command: &str, - op: &'static str, - label: &'static str, - exec_label: &'static str, - clone_deadline: time::Instant, - credential_context: CredentialContext, - auth_url: Option<&fabro_redact::DisplaySafeUrl>, - ) -> Result<(), DockerCloneFailure> { - let plan = git_retry::RetryPlan::clone_default(Some(clone_deadline)); - git_retry::retry_git_operation( - SandboxProviderKind::DOCKER, - op, - &plan, - |_attempt| async move { - let remaining = clone_deadline.saturating_duration_since(time::Instant::now()); - let timeout_ms = u64::try_from(remaining.as_millis()).unwrap_or(u64::MAX); - if timeout_ms == 0 { - return Err(DockerCloneFailure { - error: crate::Error::message(format!( - "{label} deadline expired before retry" - )), - retry_reason: None, - }); - } - let result = self - .docker_exec_shell_streaming(ExecStreamingRequest { - timeout_ms: Some(timeout_ms), - working_dir: Some("/"), - ..ExecStreamingRequest::new(command) - }) - .await - .map_err(|error| DockerCloneFailure { - error: crate::Error::context( - format!("{label} transport failed"), - error, - ), - retry_reason: None, - })? - .result; - if result.is_success() { - return Ok(()); - } - let retry_reason = classify_docker_clone_result(&result, credential_context); - Err(DockerCloneFailure { - error: self.clone_failure_error( - result, - exec_label, - auth_url, - CloneStep::Network, - ), - retry_reason, - }) - }, - |failure: &DockerCloneFailure| failure.retry_reason, - ) - .await - } - - async fn clone_github_repo( - &self, - origin_url: String, - branch: Option, - tag: Option, - commit_sha: Option, - ) -> crate::Result<()> { - self.verify_git_available().await?; - let layout = clone_source::github_repo_layout(&origin_url, WORKING_DIRECTORY, REPOS_ROOT)?; - // The clone mints its own token (never a warm-cache reuse) and seeds - // the shared source, so the first refresh compares against the clone - // token instead of believing nothing was ever embedded. - let resolved_token = match self.push_credentials.source() { - Some(source) => Some(source.mint_for_clone().await.map_err(|err| { - crate::Error::context_anyhow("Failed to get GitHub App credentials for clone", err) - })?), - None => None, - }; - // The clone call site maps its mint knowledge onto the credential - // context: a token minted for this clone is FreshApp; a static - // credential cannot become valid by waiting. - let clone_credential_context = - CredentialContext::from_snapshot(resolved_token.as_ref().map(|token| &token.snapshot)); - - let auth_url = match &resolved_token { - Some(token) => Some( - fabro_github::embed_token_in_url(&origin_url, token.token.expose()).map_err( - |err| { - crate::Error::context_anyhow( - "Failed to build authenticated GitHub clone URL", - err, - ) - }, - )?, - ), - None => None, - }; - let clone_url = auth_url - .as_ref() - .map_or(origin_url.as_str(), |url| url.as_raw_url().as_str()); - - self.emit(SandboxEvent::GitCloneStarted { - url: origin_url.clone(), - branch: branch.clone(), - }); - let clone_start = Instant::now(); - - let prepare_command = format!( - "mkdir -p {} {}", - shell_quote(WORKING_DIRECTORY), - shell_quote(&layout.repos_owner_path), - ); - match self - .docker_exec_shell(&prepare_command, 10_000, Some("/"), None, None) - .await - { - Ok(result) if result.is_success() => {} - Ok(result) => { - let err = result.into_exec_error("prepare Docker repository checkout"); - return Err(self.report_clone_failure(&origin_url, err)); - } - Err(err) => { - return Err(self.report_clone_failure(&origin_url, err)); - } - } - - let clone_deadline = time::Instant::now() + GIT_CLONE_TIMEOUT; - if let Some(pin) = - clone_source::PinnedRevision::from_selectors(tag.as_deref(), commit_sha.as_deref()) - { - // `decide_clone` already rejects a pinned revision without a - // branch; re-check here so the checkout can never silently drop the - // branch name callers read back out of the workspace. - let Some(branch) = branch.as_deref().filter(|branch| !branch.trim().is_empty()) else { - let error = - crate::Error::message(format!("{} requires a repository branch", pin.label())); - return Err(self.report_clone_failure(&origin_url, error)); - }; - - let init_command = - clone_source::exact_repository_init_command(clone_url, &layout.primary_repo_path); - if let Err(error) = self - .run_exact_local_git_command( - &init_command, - "initialize Docker pinned repository checkout", - clone_deadline, - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, error)); - } - - let fetch_command = clone_source::pinned_fetch_command( - &layout.primary_repo_path, - "origin", - &pin.fetch_refspec(), - self.config.clone_depth, - ); - if let Err(failure) = self - .retry_git_transfer( - &fetch_command, - "fetch", - "Docker pinned fetch", - "git fetch pinned revision", - clone_deadline, - clone_credential_context, - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, failure.error)); - } - - let checkout_command = clone_source::exact_checkout_verify_command( - &layout.primary_repo_path, - branch, - clone_source::FETCH_HEAD_COMMIT, - ); - let head = match self - .run_exact_local_git_command( - &checkout_command, - "git checkout pinned revision", - clone_deadline, - auth_url.as_ref(), - ) - .await - { - Ok(result) => result, - Err(error) => return Err(self.report_clone_failure(&origin_url, error)), - }; - if let Err(error) = pin.verify_head(&head.stdout) { - return Err(self.report_clone_failure(&origin_url, error)); - } - } else { - let command = git_clone_command( - clone_url, - branch.as_deref(), - &layout.primary_repo_path, - self.config.clone_depth, - ); - if let Err(failure) = self - .retry_git_transfer( - &command, - "clone", - "Docker git clone", - "git clone", - clone_deadline, - clone_credential_context, - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, failure.error)); - } - } - - let symlink_command = clone_source::repo_symlink_command(&layout); - match self - .docker_exec_shell(&symlink_command, 10_000, Some("/"), None, None) - .await - { - Ok(result) if result.is_success() => {} - Ok(result) => { - let err = result.into_exec_error("create Docker workspace repo symlink"); - return Err(self.report_clone_failure(&origin_url, err)); - } - Err(err) => { - return Err(self.report_clone_failure(&origin_url, err)); - } - } - - let _ = self.repo_cloned.set(true); - let _ = self.origin_url.set(origin_url.clone()); - self.set_working_directory(layout.execution_directory.clone())?; - if let Some(token) = resolved_token { - // The clone URL embedded this token in `origin`; record it so - // refreshes compare against the clone generation. - self.push_credentials.record_embedded(token).await; - } - - if let Some(auth_url) = auth_url.as_ref() { - let command = format!( - "git -c maintenance.auto=0 remote set-url origin {}", - shell_quote(auth_url.as_raw_url().as_str()) - ); - let result = self - .docker_exec_shell( - &command, - 10_000, - Some(&layout.execution_directory), - None, - None, - ) - .await?; - if !result.is_success() { - let err = result - .into_exec_error_with_redactor("git remote set-url origin (post-clone)", |s| { - redact_auth_url(s, Some(auth_url)) - }); - tracing::warn!( - error = %err, - "Failed to set Docker sandbox push credentials on origin — \ - subsequent git push from this sandbox will fail" - ); - } - } - - let clone_duration = u64::try_from(clone_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::GitCloneCompleted { - url: origin_url, - duration_ms: clone_duration, - }); - Ok(()) - } - - async fn validate_managed_container(&self, container_id: &str) -> crate::Result<()> { - let labels = self.inspect_labels(container_id).await?; - verify_managed_labels(container_id, &labels, self.run_id.as_ref()) - } - - async fn inspect_container( - &self, - container_id: &str, - ) -> crate::Result { - self.docker - .inspect_container(container_id, None::) - .await - .map_err(|source| { - let message = if docker_not_found(&source) { - format!("Docker container '{container_id}' is gone") - } else { - format!("Failed to inspect Docker container '{container_id}'") - }; - crate::Error::context(message, source) - }) - } - - async fn inspect_labels(&self, container_id: &str) -> crate::Result> { - let inspect = self.inspect_container(container_id).await?; - Ok(container_labels(&inspect)) - } - - async fn ensure_name_available(&self) -> crate::Result> { - let Some(run_id) = self.run_id.as_ref() else { - return Ok(None); - }; - let name = container_name(run_id); - match self - .docker - .inspect_container(&name, None::) - .await - { - Ok(_) => Err(crate::Error::message(format!( - "Docker container name '{name}' already exists for run {run_id}. Remove the stale container manually before retrying." - ))), - Err(e) if docker_not_found(&e) => Ok(Some(name)), - Err(e) => Err(crate::Error::message(format!( - "Failed to check Docker container name '{name}' before creation: {e}" - ))), - } - } - - async fn upload_bytes_to_container(&self, path: &str, bytes: &[u8]) -> crate::Result<()> { - let container_path = self.resolve_container_path(path); - let container_id = self.container_id()?; - let parent_dir = std::path::Path::new(&container_path) - .parent() - .map_or_else(|| "/".to_string(), |p| p.to_string_lossy().to_string()); - let file_name = std::path::Path::new(&container_path) - .file_name() - .ok_or_else(|| crate::Error::message(format!("Invalid path: {container_path}")))? - .to_string_lossy() - .to_string(); - - // Fabro runtime files stay owner-private; repository files keep the - // conventional world-readable mode. - let is_runtime_path = container_path.starts_with(&format!("{RUNTIME_DIRECTORY}/")); - let mkdir_cmd = if is_runtime_path { - format!("umask 077 && mkdir -p {}", shell_quote(&parent_dir)) - } else { - format!("mkdir -p {}", shell_quote(&parent_dir)) - }; - let result = self - .docker_exec_shell(&mkdir_cmd, 10_000, Some("/"), None, None) - .await?; - if !result.is_success() { - return Err(crate::Error::message(format!( - "Failed to create parent dirs for {container_path}: {}", - result.stderr - ))); - } - - let file_mode = if is_runtime_path { 0o600 } else { 0o644 }; - let tar_bytes = build_single_file_tar(&file_name, bytes, file_mode)?; - let upload_opts = UploadToContainerOptions { - path: parent_dir, - no_overwrite_dir_non_dir: "false".to_string(), - }; - - self.docker - .upload_to_container(container_id, Some(upload_opts), tar_bytes.into()) - .await - .map_err(|e| crate::Error::context("Failed to upload file to container", e)) - } - - fn begin_start(&self) -> Instant { - self.emit(SandboxEvent::StartStarted { - provider: "docker".into(), - }); - Instant::now() - } - - async fn set_container_running( - &self, - container_id: &str, - labels: &HashMap, - action: ContainerStartAction, - ) -> crate::Result<()> { - let result = match action { - ContainerStartAction::Start => { - self.docker - .start_container(container_id, None::>) - .await - } - ContainerStartAction::Unpause => self.docker.unpause_container(container_id).await, - }; - if let Err(source) = result { - if !docker_not_modified(&source) { - return Err(crate::Error::context( - format!( - "Failed to {action} Docker container '{container_id}' with labels {labels:?}" - ), - source, - )); - } - } - Ok(()) - } - - async fn complete_start( - &self, - started: Instant, - container_id: &str, - labels: &HashMap, - action: ContainerStartAction, - ) -> crate::Result<()> { - if let Err(error) = self - .set_container_running(container_id, labels, action) - .await - { - return self.start_error(error); - } - if let Err(error) = self.probe_bash(None).await { - return self.start_error(crate::Error::context( - format!("Docker container '{container_id}' health check"), - error, - )); - } - - self.emit(SandboxEvent::StartCompleted { - provider: "docker".into(), - duration_ms: elapsed_ms(started), - }); - Ok(()) - } - - fn start_error(&self, error: crate::Error) -> crate::Result<()> { - self.emit(SandboxEvent::StartFailed { - provider: "docker".into(), - error: error.to_string(), - causes: error.causes(), - }); - Err(error) - } - - fn stop_error(&self, error: crate::Error) -> crate::Result<()> { - self.emit(SandboxEvent::StopFailed { - provider: "docker".into(), - error: error.to_string(), - causes: error.causes(), - }); - Err(error) - } - - fn delete_error(&self, error: crate::Error) -> crate::Result<()> { - self.emit(SandboxEvent::DeleteFailed { - provider: "docker".into(), - error: error.to_string(), - causes: error.causes(), - }); - Err(error) - } - - fn fail_init(&self, init_start: Instant, err: crate::Error) -> crate::Error { - let duration_ms = u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::InitializeFailed { - provider: "docker".into(), - error: err.to_string(), - causes: err.causes(), - duration_ms, - }); - err +/// The workspace layout every Docker sandbox uses. +pub(crate) fn layout() -> WorkspaceLayout { + WorkspaceLayout { + workspace_root: WORKING_DIRECTORY.to_string(), + repos_root: REPOS_ROOT.to_string(), } } -fn container_name(run_id: &RunId) -> String { +pub(crate) fn container_name(run_id: &RunId) -> String { format!("fabro-run-{run_id}") } -fn docker_exec_control_paths() -> (String, String) { - let sequence = EXEC_CONTROL_COUNTER.fetch_add(1, Ordering::Relaxed); - let nonce = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_nanos()) - .unwrap_or_default(); - let prefix = format!("/tmp/fabro-exec-{}-{nonce}-{sequence}", std::process::id()); - (format!("{prefix}.stop"), format!("{prefix}.pid")) +/// The driver spec for a fabro Docker sandbox. +pub(crate) fn driver_spec(options: &DockerSandboxOptions, run_id: Option<&RunId>) -> DriverSpec { + let mut spec = DriverSpec::new(SandboxSource::Image { + reference: options.image.clone(), + }) + .working_directory(WORKING_DIRECTORY) + .network(match options.network_mode.as_deref() { + Some("none") => NetworkPolicy::Block, + _ => NetworkPolicy::AllowAll, + }) + .provider_config( + DockerProviderConfig { + auto_pull: options.auto_pull, + ..DockerProviderConfig::default() + } + .into_value(), + ); + if let Some(run_id) = run_id { + spec = spec.name(container_name(run_id)); + } + for (key, value) in managed_labels::for_run(run_id) { + spec = spec.label(key, value); + } + for entry in &options.env_vars { + let (key, value) = entry.split_once('=').unwrap_or((entry.as_str(), "")); + spec = spec.env_var(key, value); + } + let mut resources = Resources::default(); + resources.cpu_cores = options + .cpu_quota + .filter(|quota| *quota > 0) + .map(|quota| (quota + CPU_PERIOD_MICROS - 1) / CPU_PERIOD_MICROS) + .and_then(|cores| u32::try_from(cores).ok()); + resources.memory_mb = options + .memory_limit + .filter(|bytes| *bytes > 0) + .and_then(|bytes| u64::try_from(bytes).ok()) + .map(|bytes| bytes.div_ceil(1024 * 1024)); + spec.resources(resources) } -fn docker_controlled_shell_command(command: &str, stop_file: &str, pid_file: &str) -> String { - format!( - "\ -stop_file={stop_file}; \ -pid_file={pid_file}; \ -user_command={command}; \ -exec 3<&0; \ -rm -f \"$pid_file\"; \ -if [ -e \"$stop_file\" ]; then \ - rm -f \"$stop_file\" \"$pid_file\"; \ - exit 143; \ -fi; \ -( \ - while [ ! -e \"$stop_file\" ]; do sleep {stop_poll_sleep}; done; \ - while [ ! -s \"$pid_file\" ]; do sleep {stop_poll_sleep}; done; \ - child=$(cat \"$pid_file\"); \ - kill -TERM \"-$child\" 2>/dev/null || kill -TERM \"$child\" 2>/dev/null || true; \ - sleep {term_grace}; \ - kill -KILL \"-$child\" 2>/dev/null || kill -KILL \"$child\" 2>/dev/null || true; \ -) & watcher=$!; \ -if command -v setsid >/dev/null 2>&1; then \ - setsid {bash} -c \"$user_command\" <&3 & \ -else \ - {bash} -c \"$user_command\" <&3 & \ -fi; \ -child=$!; \ -exec 3<&-; \ -echo \"$child\" > \"$pid_file\"; \ -wait \"$child\"; \ -status=$?; \ -kill \"$watcher\" 2>/dev/null || true; \ -wait \"$watcher\" 2>/dev/null || true; \ -rm -f \"$stop_file\" \"$pid_file\"; \ -exit \"$status\"\ -", - bash = REMOTE_BASH, - stop_file = shell_quote(stop_file), - pid_file = shell_quote(pid_file), - command = shell_quote(command), - stop_poll_sleep = EXEC_STOP_POLL_SLEEP_SECONDS, - term_grace = EXEC_TERM_GRACE_SECONDS, +async fn connect_docker() -> crate::Result> { + connect_provider( + &SandboxProviderKind::DOCKER, + &ServerSandboxProviderSettings::default(), + &ProviderConnectOptions::default(), ) + .await + .map(|connected| connected.provider) + .map_err(|error| crate::Error::context("Failed to connect to the Docker provider", error)) } -fn docker_stdio_exec_options( - command: String, - working_dir: String, - env: Option>, -) -> (CreateExecOptions, StartExecOptions) { - let env = clean_bash_env_entries(env.unwrap_or_default()); - ( - CreateExecOptions { - attach_stdin: Some(true), - attach_stdout: Some(true), - attach_stderr: Some(true), - tty: Some(false), - cmd: Some(vec![REMOTE_BASH.to_string(), "-c".to_string(), command]), - working_dir: Some(working_dir), - env: Some(env), - ..Default::default() - }, - StartExecOptions { - detach: false, - tty: false, - output_capacity: None, - }, - ) +/// A Docker sandbox for a run. The container is created by `initialize`; +/// construction validates the clone request and connects the provider, so a +/// bad spec fails before any daemon call. +pub async fn docker_sandbox( + options: DockerSandboxOptions, + github_app: Option<&GitHubCredentials>, + run_id: Option, + clone_origin_url: Option, + clone_branch: Option, + clone_tag: Option, + clone_commit_sha: Option, +) -> crate::Result { + let workspace = RepoWorkspace::plan( + layout(), + options.skip_clone, + clone_origin_url.as_deref(), + clone_branch.as_deref(), + clone_tag.as_deref(), + clone_commit_sha.as_deref(), + options + .clone_depth + .and_then(|depth| u32::try_from(depth).ok()), + github_app, + )?; + let provider = connect_docker().await?; + let spec = driver_spec(&options, run_id.as_ref()); + Ok(DriverSandbox::pending( + SandboxProviderKind::DOCKER, + provider, + spec, + Some(options.image), + workspace, + )) } -async fn create_and_start_exec( - docker: &Docker, +/// Reattach to a run's Docker container by its persisted id. +/// +/// The container must carry fabro's managed label and, when a run id is +/// known, the matching run label: the driver shares a daemon with every +/// other application, and fabro never operates on a container it did not +/// create. +pub async fn attach_docker( container_id: &str, - exec_options: CreateExecOptions, - start_options: Option, - create_context: &'static str, - start_context: &'static str, -) -> crate::Result<(String, StartExecResults)> { - let exec_instance = docker - .create_exec(container_id, exec_options) - .await - .map_err(|err| crate::Error::context(create_context, err))?; - let exec_id = exec_instance.id; - let start_result = docker - .start_exec(&exec_id, start_options) - .await - .map_err(|err| crate::Error::context(start_context, err))?; - - Ok((exec_id, start_result)) + repo_cloned: bool, + working_directory: String, + clone_origin_url: Option, + run_id: Option, +) -> crate::Result { + let provider = connect_docker().await?; + let id = SandboxId::try_new(container_id) + .map_err(|error| crate::Error::context("Invalid Docker container id", error))?; + let handle = provider.attach(&id, None).await.map_err(|error| { + crate::Error::context( + format!("Failed to reconnect Docker container '{container_id}'"), + error, + ) + })?; + let status = handle.describe().await?; + verify_managed_labels(container_id, &status.labels, run_id.as_ref())?; + let workspace = + RepoWorkspace::attached(layout(), repo_cloned, working_directory, clone_origin_url); + Ok(DriverSandbox::attached( + SandboxProviderKind::DOCKER, + handle, + workspace, + )) } -fn docker_stop_request_exec_options(stop_file: &str) -> CreateExecOptions { - CreateExecOptions { - cmd: Some(vec![ - REMOTE_BASH.to_string(), - "-c".to_string(), - format!("touch {}", shell_quote(stop_file)), - ]), - attach_stdout: Some(true), - attach_stderr: Some(true), - working_dir: Some("/".to_string()), - env: Some(clean_bash_env_entries(Vec::new())), - ..Default::default() +/// Whether the Docker daemon answers. Used by `fabro doctor`. +pub async fn check_docker_daemon() -> crate::Result<()> { + let provider = connect_docker().await?; + let health = provider + .health() + .await + .map_err(|error| crate::Error::context("Docker health check failed", error))?; + match health.status { + HealthStatus::Ok | HealthStatus::Unknown => Ok(()), + HealthStatus::Unreachable | HealthStatus::Unauthorized => { + Err(crate::Error::message(health.message.unwrap_or_else(|| { + "Failed to reach Docker daemon".to_string() + }))) + } + _ => Err(crate::Error::message( + "Docker daemon reported an unknown health state", + )), } } -async fn request_docker_exec_stop_with( - docker: &Docker, +pub(crate) fn verify_managed_labels( container_id: &str, - stop_file: &str, -) -> crate::Result<()> { - let (exec_id, start_result) = create_and_start_exec( - docker, - container_id, - docker_stop_request_exec_options(stop_file), - None, - "Failed to create Docker exec stop request", - "Failed to start Docker exec stop request", - ) - .await?; - - let mut stdout = String::new(); - let mut stderr = String::new(); - if let StartExecResults::Attached { mut output, .. } = start_result { - while let Some(chunk) = output.next().await { - match chunk { - Ok(LogOutput::StdOut { message }) => { - stdout.push_str(&String::from_utf8_lossy(&message)); - } - Ok(LogOutput::StdErr { message }) => { - stderr.push_str(&String::from_utf8_lossy(&message)); - } - Ok(_) => {} - Err(e) => { - return Err(crate::Error::context( - "Error reading stop request output", - e, - )); - } - } - } - } - - let inspect = docker - .inspect_exec(&exec_id) - .await - .map_err(|e| crate::Error::context("Failed to inspect Docker exec stop request", e))?; - let exit_code = inspect - .exit_code - .and_then(|code| i32::try_from(code).ok()) - .unwrap_or(-1); - if exit_code != 0 { - return Err(crate::Error::message(format!( - "Failed to request Docker exec stop (exit {exit_code}): {stderr}{stdout}" - ))); - } - Ok(()) -} - -struct DockerStdioProcessControl { - docker: Docker, - container_id: String, - exec_id: String, - stop_file: String, - state: Arc, -} - -#[derive(Default)] -struct DockerStdioProcessState { - stop_requested: AtomicBool, - termination: TokioMutex>, - termination_notify: Notify, -} - -impl DockerStdioProcessState { - async fn cached_termination(&self) -> Option { - *self.termination.lock().await - } - - async fn request_stop_once(&self) -> bool { - self.cached_termination().await.is_none() - && !self.stop_requested.swap(true, Ordering::AcqRel) - } - - async fn cache_termination(&self, termination: StdioProcessTermination) { - let mut cached = self.termination.lock().await; - if cached.is_none() { - *cached = Some(termination); - self.termination_notify.notify_waiters(); - } - } - - async fn wait_for_cached_termination(&self) -> StdioProcessTermination { - loop { - if let Some(termination) = self.cached_termination().await { - return termination; - } - self.termination_notify.notified().await; - } - } -} - -#[async_trait] -impl StdioProcessControl for DockerStdioProcessControl { - async fn terminate(&self) -> crate::Result<()> { - if !self.state.request_stop_once().await { - return Ok(()); - } - request_docker_exec_stop_with(&self.docker, &self.container_id, &self.stop_file).await?; - Ok(()) - } - - async fn wait(&self) -> crate::Result { - if let Some(termination) = self.state.cached_termination().await { - return Ok(termination); - } - - let mut poll_interval = time::interval(std::time::Duration::from_secs(1)); - loop { - if let Some(termination) = self.state.cached_termination().await { - return Ok(termination); - } - let inspect = self - .docker - .inspect_exec(&self.exec_id) - .await - .map_err(|e| crate::Error::context("Failed to inspect Docker stdio exec", e))?; - if inspect.running != Some(true) { - let exit_code = inspect.exit_code.and_then(|code| i32::try_from(code).ok()); - let termination = StdioProcessTermination::exited(exit_code); - self.state.cache_termination(termination).await; - return Ok(termination); - } - tokio::select! { - termination = self.state.wait_for_cached_termination() => return Ok(termination), - _ = poll_interval.tick() => {} - } - } - } -} - -async fn cache_docker_stdio_completion( - docker: Docker, - exec_id: String, - state: Arc, -) { - match docker.inspect_exec(&exec_id).await { - Ok(inspect) if inspect.running != Some(true) => { - let exit_code = inspect.exit_code.and_then(|code| i32::try_from(code).ok()); - state - .cache_termination(StdioProcessTermination::exited(exit_code)) - .await; - } - Ok(_) => {} - Err(err) => { - tracing::warn!(error = %err, "Failed to inspect completed Docker stdio exec"); - } - } -} - -fn git_clone_command( - clone_url: &str, - branch: Option<&str>, - checkout_path: &str, - depth: Option, -) -> String { - let mut command = format!("{} clone", sandbox::GIT); - if let Some(branch) = branch { - command.push_str(" --branch "); - command.push_str(&shell_quote(branch)); - command.push_str(" --single-branch"); - } - command.push_str(&clone_source::depth_argument(depth)); - command.push_str(" --no-tags"); - command.push_str(" -- "); - command.push_str(&shell_quote(clone_url)); - command.push(' '); - command.push_str(&shell_quote(checkout_path)); - command -} - -fn classify_docker_clone_result( - result: &ExecResult, - cred: CredentialContext, -) -> Option { - git_retry::classify_output(&result.stderr, &result.stdout, cred).retry_reason() -} - -fn host_config(config: &DockerSandboxOptions) -> HostConfig { - HostConfig { - binds: None, - network_mode: config.network_mode.clone(), - memory: config.memory_limit, - cpu_quota: config.cpu_quota, - ..Default::default() - } -} - -fn container_config(config: &DockerSandboxOptions, run_id: Option<&RunId>) -> Config { - Config { - image: Some(config.image.clone()), - cmd: Some(vec![ - REMOTE_BASH.to_string(), - "-c".to_string(), - format!( - "mkdir -p {} && sleep infinity", - shell_quote(WORKING_DIRECTORY) - ), - ]), - working_dir: Some(WORKING_DIRECTORY.to_string()), - env: Some(clean_bash_env_entries(config.env_vars.clone())), - labels: Some(managed_labels::for_run(run_id)), - host_config: Some(host_config(config)), - ..Default::default() - } -} - -fn verify_managed_labels( - container_id: &str, - labels: &HashMap, + labels: &BTreeMap, run_id: Option<&RunId>, ) -> crate::Result<()> { - if labels.get(MANAGED_LABEL).map(String::as_str) != Some("true") { + if labels.get(MANAGED_LABEL).map(String::as_str) != Some(MANAGED_LABEL_VALUE) { return Err(crate::Error::message(format!( - "Refusing to operate on Docker container '{container_id}' because it is missing label {MANAGED_LABEL}=true" + "Refusing to operate on Docker container '{container_id}' because it is missing label {MANAGED_LABEL}={MANAGED_LABEL_VALUE}" ))); } if let Some(run_id) = run_id { @@ -1664,1139 +239,13 @@ fn verify_managed_labels( Ok(()) } -fn container_labels(inspect: &ContainerInspectResponse) -> HashMap { - inspect - .config - .as_ref() - .and_then(|config| config.labels.clone()) - .unwrap_or_default() -} - -fn activation_action(inspect: &ContainerInspectResponse) -> Option { - let Some(state) = inspect.state.as_ref() else { - return Some(ContainerStartAction::Start); - }; - if state.running != Some(true) { - return Some(ContainerStartAction::Start); - } - (state.paused == Some(true)).then_some(ContainerStartAction::Unpause) -} - -fn docker_not_found(error: &DockerError) -> bool { - matches!(error, DockerError::DockerResponseServerError { - status_code: 404, - .. - }) -} - -fn docker_not_modified(error: &DockerError) -> bool { - matches!(error, DockerError::DockerResponseServerError { - status_code: 304, - .. - }) -} - -fn bash_remediation(image: &str) -> String { - format!("Failed to start Docker container from image '{image}'. {DOCKER_BASH_REQUIREMENT}") -} - -fn build_single_file_tar(file_name: &str, bytes: &[u8], mode: u32) -> crate::Result> { - let mut tar_builder = tar::Builder::new(Vec::new()); - let mut header = tar::Header::new_gnu(); - header - .set_path(file_name) - .map_err(|e| crate::Error::context("Failed to set tar path", e))?; - header.set_size( - u64::try_from(bytes.len()) - .map_err(|_| crate::Error::message("file is too large for tar header"))?, - ); - header.set_mode(mode); - header.set_cksum(); - tar_builder - .append(&header, bytes) - .map_err(|e| crate::Error::context("Failed to build tar archive", e))?; - tar_builder - .into_inner() - .map_err(|e| crate::Error::context("Failed to finalize tar archive", e)) -} - -#[async_trait] -impl Sandbox for DockerSandbox { - async fn download_file_to_local( - &self, - remote_path: &str, - local_path: &std::path::Path, - ) -> crate::Result<()> { - let bytes = self.download_file_bytes(remote_path).await?; - if let Some(parent) = local_path.parent() { - fs::create_dir_all(parent) - .await - .map_err(|e| crate::Error::context("Failed to create parent dirs", e))?; - } - fs::write(local_path, bytes).await.map_err(|e| { - crate::Error::context(format!("Failed to write {}", local_path.display()), e) - }) - } - - async fn upload_file_from_local( - &self, - local_path: &std::path::Path, - remote_path: &str, - ) -> crate::Result<()> { - let bytes = fs::read(local_path).await.map_err(|e| { - crate::Error::context(format!("Failed to read {}", local_path.display()), e) - })?; - self.upload_bytes_to_container(remote_path, &bytes).await - } - - async fn initialize(&self) -> crate::Result<()> { - self.emit(SandboxEvent::Initializing { - provider: "docker".into(), - }); - let init_start = Instant::now(); - - let pull_start = Instant::now(); - match self.ensure_image().await { - Ok(EnsureImageOutcome::Skipped) => {} - Ok(EnsureImageOutcome::AlreadyLocal | EnsureImageOutcome::Pulled) => { - let pull_duration = - u64::try_from(pull_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::SnapshotReady { - name: self.config.image.clone(), - duration_ms: pull_duration, - }); - } - Err(e) => { - self.emit(SandboxEvent::SnapshotFailed { - name: self.config.image.clone(), - error: e.to_string(), - causes: e.causes(), - }); - return Err(self.fail_init(init_start, e)); - } - } - - let container_name = self - .ensure_name_available() - .await - .map_err(|e| self.fail_init(init_start, e))?; - let create_options = container_name.map(|name| CreateContainerOptions { - name, - platform: None, - }); - let container = self - .docker - .create_container(create_options, container_config(&self.config, self.run_id.as_ref())) - .await - .map_err(|e| { - let message = if matches!( - e, - DockerError::DockerResponseServerError { - status_code: 409, - .. - } - ) { - "Docker container for run already exists. Remove the stale fabro-run container manually before retrying.".to_string() - } else { - "Failed to create Docker container".to_string() - }; - self.fail_init(init_start, crate::Error::context(message, e)) - })?; - - let id = container.id.clone(); - self.container_id - .set(id.clone()) - .map_err(|_| crate::Error::message("Container already initialized"))?; - - self.docker - .start_container(&id, None::>) - .await - .map_err(|e| { - let err = crate::Error::context(bash_remediation(&self.config.image), e); - self.fail_init(init_start, err) - })?; - - if let Err(e) = self.probe_bash(Some(WORKING_DIRECTORY)).await { - return Err(self.fail_init(init_start, e)); - } - - let (uname_output, _, _) = self - .docker_exec(vec!["uname".to_string(), "-r".to_string()], None, None) - .await?; - let _ = self.cached_platform.set("linux".to_string()); - let _ = self - .cached_os_version - .set(format!("linux {}", uname_output.trim())); - - if let Err(e) = self.create_runtime_directory().await { - return Err(self.fail_init(init_start, e)); - } - - let clone_decision = clone_source::decide_clone( - self.config.skip_clone, - self.clone_origin_url.as_deref(), - self.clone_branch.as_deref(), - self.clone_tag.as_deref(), - self.clone_commit_sha.as_deref(), - ) - .map_err(|e| self.fail_init(init_start, e))?; - - match clone_decision { - CloneDecision::EmptyWorkspace { reason } => { - if matches!(reason, EmptyWorkspaceReason::MissingOrigin) { - tracing::warn!( - provider = "docker", - reason = reason.message(), - "Clone source missing for clone-based sandbox" - ); - } - if let Err(e) = self.create_workspace().await { - return Err(self.fail_init(init_start, e)); - } - let _ = self.repo_cloned.set(false); - } - CloneDecision::GitHub { - origin_url, - branch, - tag, - commit_sha, - } => { - if let Err(e) = self - .clone_github_repo(origin_url, branch, tag, commit_sha) - .await - { - return Err(self.fail_init(init_start, e)); - } - } - } - - let init_duration = u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::Ready { - provider: "docker".into(), - duration_ms: init_duration, - name: None, - cpu: None, - memory: None, - url: None, - }); - - Ok(()) - } - - async fn start(&self) -> crate::Result<()> { - let started = self.begin_start(); - let container_id = self.container_id()?.to_string(); - let inspect = match self.inspect_container(&container_id).await { - Ok(inspect) => inspect, - Err(error) => return self.start_error(error), - }; - let labels = container_labels(&inspect); - if let Err(error) = verify_managed_labels(&container_id, &labels, self.run_id.as_ref()) { - return self.start_error(error); - } - let action = activation_action(&inspect).unwrap_or(ContainerStartAction::Start); - self.complete_start(started, &container_id, &labels, action) - .await - } - - async fn activate(&self) -> crate::Result<()> { - let container_id = self.container_id()?.to_string(); - let inspect = self.inspect_container(&container_id).await?; - let labels = container_labels(&inspect); - verify_managed_labels(&container_id, &labels, self.run_id.as_ref())?; - let Some(action) = activation_action(&inspect) else { - return Ok(()); - }; - match action { - ContainerStartAction::Unpause => { - self.set_container_running(&container_id, &labels, action) - .await - } - ContainerStartAction::Start => { - let started = self.begin_start(); - self.complete_start(started, &container_id, &labels, action) - .await - } - } - } - - async fn stop(&self) -> crate::Result<()> { - self.emit(SandboxEvent::StopStarted { - provider: "docker".into(), - }); - let start = Instant::now(); - - let Some(container_id) = self.container_id.get().cloned() else { - let duration_ms = elapsed_ms(start); - self.emit(SandboxEvent::StopCompleted { - provider: "docker".into(), - duration_ms, - }); - return Ok(()); - }; - - let labels = match self.inspect_labels(&container_id).await { - Ok(labels) => labels, - Err(e) => return self.stop_error(e), - }; - if let Err(e) = verify_managed_labels(&container_id, &labels, self.run_id.as_ref()) { - return self.stop_error(e); - } - - let stop_opts = StopContainerOptions { t: 1 }; - if let Err(e) = self - .docker - .stop_container(&container_id, Some(stop_opts)) - .await - { - if !docker_not_found(&e) && !docker_not_modified(&e) { - return self.stop_error(crate::Error::context( - format!( - "Failed to stop Docker container '{container_id}' with labels {labels:?}" - ), - e, - )); - } - } - - let duration_ms = elapsed_ms(start); - self.emit(SandboxEvent::StopCompleted { - provider: "docker".into(), - duration_ms, - }); - - Ok(()) - } - - async fn delete(&self) -> crate::Result<()> { - self.emit(SandboxEvent::DeleteStarted { - provider: "docker".into(), - }); - let start = Instant::now(); - - let Some(container_id) = self.container_id.get().cloned() else { - let duration_ms = elapsed_ms(start); - self.emit(SandboxEvent::DeleteCompleted { - provider: "docker".into(), - duration_ms, - }); - return Ok(()); - }; - - let labels = match self.inspect_labels(&container_id).await { - Ok(labels) => labels, - Err(e) => return self.delete_error(e), - }; - if let Err(e) = verify_managed_labels(&container_id, &labels, self.run_id.as_ref()) { - return self.delete_error(e); - } - - let remove_opts = RemoveContainerOptions { - force: true, - ..Default::default() - }; - if let Err(e) = self - .docker - .remove_container(&container_id, Some(remove_opts)) - .await - { - if !docker_not_found(&e) { - return self.delete_error(crate::Error::context( - format!( - "Failed to remove Docker container '{container_id}' with labels {labels:?}" - ), - e, - )); - } - } - - let duration_ms = elapsed_ms(start); - self.emit(SandboxEvent::DeleteCompleted { - provider: "docker".into(), - duration_ms, - }); - - Ok(()) - } - - async fn cleanup(&self) -> crate::Result<()> { - self.delete().await - } - - async fn exec_command( - &self, - command: &str, - timeout_ms: u64, - working_dir: Option<&str>, - env_vars: Option<&HashMap>, - cancel_token: Option, - ) -> crate::Result { - let dir = working_dir.map(|path| self.resolve_container_path(path)); - self.docker_exec_shell(command, timeout_ms, dir.as_deref(), env_vars, cancel_token) - .await - } - - async fn exec_command_streaming( - &self, - request: ExecStreamingRequest<'_>, - ) -> crate::Result { - let dir = request - .working_dir - .map(|path| self.resolve_container_path(path)); - self.docker_exec_shell_streaming(ExecStreamingRequest { - working_dir: dir.as_deref(), - ..request - }) - .await - } - - async fn spawn_stdio_process( - &self, - command: &str, - working_dir: Option<&str>, - env_vars: Option<&HashMap>, - cancel_token: Option, - ) -> crate::Result { - let effective_dir = working_dir.map_or_else( - || self.working_directory().to_string(), - |path| self.resolve_container_path(path), - ); - let env = env_vars.map(|vars| { - vars.iter() - .map(|(key, value)| format!("{key}={value}")) - .collect() - }); - let (stop_file, pid_file) = docker_exec_control_paths(); - let controlled_command = docker_controlled_shell_command(command, &stop_file, &pid_file); - let (create_opts, start_opts) = - docker_stdio_exec_options(controlled_command, effective_dir, env); - - let container_id = self.container_id()?.to_string(); - let (exec_id, start_result) = create_and_start_exec( - &self.docker, - &container_id, - create_opts, - Some(start_opts), - "Failed to create Docker stdio exec", - "Failed to start Docker stdio exec", - ) - .await?; - - let StartExecResults::Attached { mut output, input } = start_result else { - return Err(crate::Error::message( - "Docker stdio exec started detached unexpectedly", - )); - }; - - let stderr_collector = StderrCollector::new(DEFAULT_EXEC_OUTPUT_TAIL_BYTES); - let stderr_for_output = stderr_collector.clone(); - let (mut stdout_writer, stdout_reader) = duplex(64 * 1024); - let state = Arc::new(DockerStdioProcessState::default()); - let state_for_output = Arc::clone(&state); - let docker_for_output = self.docker.clone(); - let exec_id_for_output = exec_id.clone(); - tokio::spawn(async move { - while let Some(chunk) = output.next().await { - match chunk { - Ok(LogOutput::StdOut { message }) => { - if let Err(err) = stdout_writer.write_all(&message).await { - tracing::warn!(error = %err, "Failed to forward Docker stdio stdout"); - break; - } - } - Ok(LogOutput::StdErr { message }) => { - stderr_for_output.push(&message).await; - } - Ok(_) => {} - Err(err) => { - let message = format!("Docker stdio output stream error: {err}"); - stderr_for_output.push(message.as_bytes()).await; - break; - } - } - } - cache_docker_stdio_completion(docker_for_output, exec_id_for_output, state_for_output) - .await; - }); - - let handle = StdioProcessHandle::new(DockerStdioProcessControl { - docker: self.docker.clone(), - container_id, - exec_id, - stop_file, - state, - }); - - if let Some(token) = cancel_token { - let handle_for_cancel = handle.clone(); - tokio::spawn(async move { - token.cancelled().await; - if let Err(err) = handle_for_cancel.terminate().await { - tracing::warn!(error = %err, "Failed to terminate cancelled Docker stdio exec"); - } - }); - } - - Ok(StdioProcess { - stdin: input, - stdout: Box::pin(stdout_reader), - stderr: stderr_collector, - handle, - }) - } - - async fn read_file_bytes(&self, path: &str) -> crate::Result> { - self.download_file_bytes(path).await - } - - async fn read_file( - &self, - path: &str, - offset: Option, - limit: Option, - ) -> crate::Result { - let container_path = self.resolve_container_path(path); - let (stdout, stderr, exit_code) = self - .docker_exec(vec!["cat".to_string(), container_path.clone()], None, None) - .await?; - - if exit_code != 0 { - return Err(crate::Error::message(format!( - "Failed to read {container_path}: {stderr}" - ))); - } - - Ok(format_lines_numbered(&stdout, offset, limit)) - } - - async fn write_file(&self, path: &str, content: &str) -> crate::Result<()> { - self.upload_bytes_to_container(path, content.as_bytes()) - .await - } - - async fn delete_file(&self, path: &str) -> crate::Result<()> { - let container_path = self.resolve_container_path(path); - let (_, stderr, exit_code) = self - .docker_exec( - vec!["rm".to_string(), "-f".to_string(), container_path.clone()], - None, - None, - ) - .await?; - - if exit_code != 0 { - return Err(crate::Error::message(format!( - "Failed to delete {container_path}: {stderr}" - ))); - } - Ok(()) - } - - async fn file_exists(&self, path: &str) -> crate::Result { - let container_path = self.resolve_container_path(path); - let (_, _, exit_code) = self - .docker_exec( - vec!["test".to_string(), "-e".to_string(), container_path], - None, - None, - ) - .await?; - - Ok(exit_code == 0) - } - - async fn list_directory( - &self, - path: &str, - depth: Option, - ) -> crate::Result> { - let container_path = self.resolve_container_path(path); - let max_depth = depth.unwrap_or(1); - let (stdout, stderr, exit_code) = self - .docker_exec( - vec![ - "find".to_string(), - container_path.clone(), - "-mindepth".to_string(), - "1".to_string(), - "-maxdepth".to_string(), - max_depth.to_string(), - "-printf".to_string(), - "%y\t%s\t%P\n".to_string(), - ], - None, - None, - ) - .await?; - - if exit_code != 0 { - return Err(crate::Error::message(format!( - "Failed to list directory {container_path}: {stderr}" - ))); - } - - let mut entries: Vec = stdout - .lines() - .filter(|line| !line.is_empty()) - .filter_map(|line| { - let parts: Vec<&str> = line.splitn(3, '\t').collect(); - if parts.len() < 3 { - return None; - } - let file_type = parts[0]; - let size: Option = parts[1].parse().ok(); - let name = parts[2].to_string(); - let is_dir = file_type == "d"; - Some(DirEntry { - name, - is_dir, - size: if is_dir { None } else { size }, - }) - }) - .collect(); - - entries.sort_by(|a, b| a.name.cmp(&b.name)); - Ok(entries) - } - - async fn grep( - &self, - pattern: &str, - path: &str, - options: &GrepOptions, - ) -> crate::Result> { - let container_path = self.resolve_container_path(path); - let use_rg = *self - .rg_available - .get_or_init(|| async { - let result = self - .docker_exec(vec!["which".to_string(), "rg".to_string()], None, None) - .await; - matches!(result, Ok((_, _, 0))) - }) - .await; - - let command = if use_rg { - let mut command = "rg -n".to_string(); - if options.case_insensitive { - command.push_str(" -i"); - } - if let Some(ref glob_filter) = options.glob_filter { - command.push_str(" --glob "); - command.push_str(&shell_quote(glob_filter)); - } - if let Some(max) = options.max_results { - let _ = write!(&mut command, " -m {max}"); - } - command.push_str(" -- "); - command.push_str(&shell_quote(pattern)); - command.push(' '); - command.push_str(&shell_quote(&container_path)); - command - } else { - let mut command = "grep -rn".to_string(); - if options.case_insensitive { - command.push_str(" -i"); - } - if let Some(ref glob_filter) = options.glob_filter { - command.push_str(" --include "); - command.push_str(&shell_quote(glob_filter)); - } - if let Some(max) = options.max_results { - let _ = write!(&mut command, " -m {max}"); - } - command.push_str(" -- "); - command.push_str(&shell_quote(pattern)); - command.push(' '); - command.push_str(&shell_quote(&container_path)); - command - }; - - let result = self - .docker_exec_shell(&command, 30_000, None, None, None) - .await?; - if result.exit_code == Some(1) { - return Ok(Vec::new()); - } - if !result.is_success() { - return Err(crate::Error::message(format!( - "grep failed (exit {}): {}", - result.display_exit_code(), - result.stderr - ))); - } - - Ok(result - .stdout - .lines() - .map(String::from) - .filter(|line| !line.is_empty()) - .collect()) - } - - async fn walk_files( - &self, - base: &str, - relative_start: &str, - options: &WalkOptions, - ) -> crate::Result> { - if options.excludes_relative_path(relative_start) { - return Ok(Vec::new()); - } - - let base = self.resolve_container_path(base); - let command = sandbox::build_remote_walk_command(&base, relative_start, options); - let result = self - .docker_exec_shell(&command, REMOTE_WALK_TIMEOUT_MS, None, None, None) - .await?; - if !result.is_success() { - return Err(crate::Error::exec("recursive file traversal", result)); - } - - sandbox::parse_remote_walk_output(&base, relative_start, &result.stdout) - } - - fn working_directory(&self) -> &str { - self.working_directory - .get() - .map_or(WORKING_DIRECTORY, String::as_str) - } - - fn runtime_directory(&self) -> Option<&str> { - Some(RUNTIME_DIRECTORY) - } - - async fn ssh_access_command(&self) -> crate::Result> { - Ok(Some(docker_access_command( - self.container_id()?, - self.working_directory(), - ))) - } - - fn platform(&self) -> &str { - self.cached_platform.get().map_or("linux", String::as_str) - } - - fn os_version(&self) -> String { - self.cached_os_version - .get() - .cloned() - .unwrap_or_else(|| "linux".to_string()) - } - - fn sandbox_info(&self) -> String { - self.container_id.get().cloned().unwrap_or_default() - } - - async fn setup_git( - &self, - intent: &crate::GitSetupIntent, - ) -> crate::Result> { - if !self.repo_cloned() { - return Ok(None); - } - crate::setup_git_via_exec(self, intent).await.map(Some) - } - - fn resume_setup_commands(&self, run_branch: &str) -> Vec { - if !self.repo_cloned() { - return Vec::new(); - } - vec![format!( - "git fetch origin {} && git checkout {}", - shell_quote(run_branch), - shell_quote(run_branch) - )] - } - - async fn git_push_ref( - &self, - refspec: &str, - plan: &crate::RetryPlan, - ) -> Result { - if !self.repo_cloned() { - return Ok(crate::PushReport::default()); - } - let credentials = self - .origin_url - .get() - .map(|origin_url| (&self.push_credentials, origin_url.as_str())); - sandbox::git_push_via_exec(self, credentials, refspec, plan).await - } - - fn origin_url(&self) -> Option<&str> { - if !self.repo_cloned() { - return None; - } - self.origin_url.get().map(String::as_str) - } - - #[tracing::instrument(name = "git_op", skip_all, fields(op = "refresh-credentials"))] - async fn refresh_push_credentials(&self) -> crate::Result { - if !self.repo_cloned() { - return Ok(RefreshOutcome::none()); - } - let Some(origin_url) = self.origin_url.get() else { - return Ok(RefreshOutcome::none()); - }; - self.push_credentials - .refresh(origin_url, |auth_url| { - push_credentials::set_auth_url_via_exec(self, auth_url) - }) - .await - } - - fn push_token_source(&self) -> Option> { - self.push_credentials.source().cloned() - } -} - #[cfg(test)] mod tests { - #[expect( - clippy::disallowed_types, - reason = "unit test reads an in-memory tar entry synchronously" - )] - use std::io::Read as _; - use std::process::Stdio; - use std::time::Duration; - - use bollard::API_DEFAULT_VERSION; - use httpmock::Method::{GET, POST}; - use httpmock::MockServer; - use tokio::io::AsyncWriteExt as _; - use tokio::process::Command; - use super::*; - use crate::sandbox::{BASH_PROBE_MARKER, bash_probe_passed}; #[test] - fn remote_walk_command_only_uses_find_for_traversal() { - let command = sandbox::build_remote_walk_command("/workspace", ".ai", &WalkOptions { - excluded_directory_names: vec!["target".to_string(), "node_modules".to_string()], - }); - - assert!(command.contains("[ ! -L /workspace/.ai ]")); - assert!(command.contains("find -H /workspace/.ai")); - assert!(command.contains("-name target")); - assert!(command.contains("-name node_modules")); - assert!(command.contains("-printf '%s\\0%P\\0'")); - assert!(!command.contains("*.md")); - } - - #[test] - fn remote_walk_output_is_relative_to_the_declared_base() { - let files = - sandbox::parse_remote_walk_output("/workspace", ".ai/reports", "12\0result.md\0") - .unwrap(); - - assert_eq!(files, vec![SandboxFile { - path: "/workspace/.ai/reports/result.md".to_string(), - relative_path: ".ai/reports/result.md".to_string(), - size: 12, - }]); - } - - #[test] - fn per_run_container_idle_command_uses_non_login_bash() { - let config = container_config(&DockerSandboxOptions::default(), None); - - assert_eq!( - config.cmd.as_ref().map(|cmd| &cmd[..2]), - Some(&["/bin/bash".to_string(), "-c".to_string()][..]) - ); - assert_eq!(config.env, Some(vec!["BASH_ENV=".to_string()])); - } - - #[test] - fn stop_request_exec_uses_non_login_bash() { - // Provider-control commands share the interpreter contract, so a - // profile file can't change how a stop request behaves. - let options = docker_stop_request_exec_options("/tmp/fabro exec.stop"); - - assert_eq!( - options.cmd, - Some(vec![ - "/bin/bash".to_string(), - "-c".to_string(), - "touch '/tmp/fabro exec.stop'".to_string(), - ]) - ); - assert_eq!(options.env, Some(vec!["BASH_ENV=".to_string()])); - } - - #[test] - fn bash_exec_env_blanks_caller_bash_env() { - let env = docker_bash_exec_env(Some(&HashMap::from([ - ( - BASH_ENV_VAR.to_string(), - "/tmp/untrusted-startup".to_string(), - ), - ("MODE".to_string(), "test".to_string()), - ]))); - - assert!(env.contains(&"MODE=test".to_string())); - assert_eq!( - env.iter() - .filter(|entry| env_entry_name(entry) == BASH_ENV_VAR) - .map(String::as_str) - .collect::>(), - vec!["BASH_ENV="] - ); - } - - #[test] - fn controlled_shell_command_starts_its_child_with_non_login_bash() { - let script = docker_controlled_shell_command("echo hi", "/tmp/stop", "/tmp/pid"); - - assert!( - script.contains("setsid /bin/bash -c \"$user_command\"") - && script.contains("/bin/bash -c \"$user_command\""), - "controlled child should run the user command under non-login bash: {script}" - ); - assert!( - !script.contains("-lc") && !script.contains("bash -l"), - "controlled shell script should contain no login invocation: {script}" - ); - } - - #[test] - fn bash_probe_is_shared_by_fresh_initialization_and_resumed_start() { - // Both lifecycle paths call `probe_bash`, so they cannot drift: a - // resumed container is checked exactly as strictly as a fresh one. - assert!(BASH_PROBE_SCRIPT.contains("BASH_VERSION")); - assert!(BASH_PROBE_SCRIPT.contains("login_shell")); - assert!( - !bash_probe_passed(Some(0), "ready"), - "a zero exit without the marker is not a successful probe" - ); - assert!(bash_probe_passed( - Some(0), - &format!("{BASH_PROBE_MARKER}\n") - )); - } - - #[tokio::test] - async fn activate_unpauses_paused_running_container() { - let server = MockServer::start_async().await; - let inspect = server - .mock_async(|when, then| { - when.method(GET) - .path_suffix("/containers/test-container/json"); - then.status(200) - .header("content-type", "application/json") - .json_body(serde_json::json!({ - "Config": { - "Labels": managed_labels::for_run(None) - }, - "State": { - "Running": true, - "Paused": true - } - })); - }) - .await; - let unpause = server - .mock_async(|when, then| { - when.method(POST) - .path_suffix("/containers/test-container/unpause"); - then.status(204); - }) - .await; - let start = server - .mock_async(|when, then| { - when.method(POST) - .path_suffix("/containers/test-container/start"); - then.status(204); - }) - .await; - let docker = Docker::connect_with_http(&server.base_url(), 5, API_DEFAULT_VERSION) - .expect("mock Docker client should connect"); - let sandbox = test_docker_sandbox(docker, "test-container"); - - sandbox - .activate() - .await - .expect("a paused running container should be unpaused"); - - inspect.assert_calls_async(1).await; - unpause.assert_calls_async(1).await; - start.assert_calls_async(0).await; - } - - #[test] - fn default_options_are_clone_based() { - let options = DockerSandboxOptions::default(); - assert_eq!(options.image, "buildpack-deps:noble"); - assert_eq!(options.network_mode.as_deref(), Some("bridge")); - assert_eq!(options.clone_depth, Some(DEFAULT_GIT_CLONE_DEPTH)); - assert!(!options.skip_clone); - } - - #[test] - fn clone_command_uses_configured_depth_without_tags_for_branch_clone() { - let command = git_clone_command( - "https://github.com/fabro-sh/fabro", - Some("main"), - "/repos/fabro-sh/fabro", - Some(1), - ); - assert_eq!( - command, - "git -c maintenance.auto=0 -c gc.auto=0 clone --branch main --single-branch --depth 1 --no-tags -- https://github.com/fabro-sh/fabro /repos/fabro-sh/fabro" - ); - } - - #[test] - fn clone_command_without_branch_retains_legacy_shape() { - let command = git_clone_command( - "https://github.com/fabro-sh/fabro", - None, - "/repos/fabro-sh/fabro", - Some(DEFAULT_GIT_CLONE_DEPTH), - ); - assert_eq!( - command, - "git -c maintenance.auto=0 -c gc.auto=0 clone --depth 100 --no-tags -- https://github.com/fabro-sh/fabro /repos/fabro-sh/fabro" - ); - } - - #[test] - fn clone_command_omits_depth_for_full_clone() { - let command = git_clone_command( - "https://github.com/fabro-sh/fabro", - Some("main"), - "/repos/fabro-sh/fabro", - None, - ); - assert_eq!( - command, - "git -c maintenance.auto=0 -c gc.auto=0 clone --branch main --single-branch --no-tags -- https://github.com/fabro-sh/fabro /repos/fabro-sh/fabro" - ); - } - - #[test] - fn invalid_exact_sha_fails_before_docker_connection() { - let error = DockerSandbox::new( - DockerSandboxOptions::default(), - None, - None, - Some("https://github.com/acme/widgets".to_string()), - Some("main".to_string()), - None, - Some("not-a-sha".to_string()), - ) - .err() - .expect("validation should run before connecting to Docker"); - - assert!(error.to_string().contains("40 ASCII hexadecimal")); - assert!(!error.to_string().contains("Docker daemon")); - } - - #[test] - fn exact_sha_without_branch_fails_before_docker_connection() { - let error = DockerSandbox::new( - DockerSandboxOptions::default(), - None, - None, - Some("https://github.com/acme/widgets".to_string()), - None, - None, - Some("0123456789abcdef0123456789abcdef01234567".to_string()), - ) - .err() - .expect("branch validation should run before connecting to Docker"); - - assert!(error.to_string().contains("requires a repository branch")); - assert!(!error.to_string().contains("Docker daemon")); - } - - #[test] - fn exact_checkout_failure_preserves_safe_source_chain() { - let docker = Docker::connect_with_http("http://127.0.0.1:2375", 5, API_DEFAULT_VERSION) - .expect("mock Docker client should connect"); - let sandbox = test_docker_sandbox(docker, "test-container"); - let token = "ghs_exact_checkout_secret"; - let auth_url = fabro_github::embed_token_in_url("https://github.com/acme/widgets", token) - .expect("authenticated URL"); - let error = sandbox.clone_failure_error( - ExecResult { - stdout: String::new(), - stderr: format!( - "fatal: unable to access {}: synthetic low-level failure", - auth_url.as_raw_url() - ), - exit_code: Some(128), - termination: CommandTermination::Exited, - duration_ms: 1, - }, - "git fetch exact commit", - Some(&auth_url), - CloneStep::Network, - ); - - let causes = error.causes(); - assert!( - causes - .iter() - .any(|cause| cause.contains("git fetch exact commit failed")), - "source chain should retain the exec failure: {causes:?}" - ); - let rendered = crate::display_for_log(&error); - assert!(!rendered.contains(token)); - assert!(!rendered.contains(auth_url.as_raw_url().as_str())); - assert!(rendered.contains("synthetic low-level failure")); - } - - #[test] - fn local_checkout_failure_does_not_blame_github_credentials() { - let docker = Docker::connect_with_http("http://127.0.0.1:2375", 5, API_DEFAULT_VERSION) - .expect("mock Docker client should connect"); - let sandbox = test_docker_sandbox(docker, "test-container"); - let failure = ExecResult { - stdout: String::new(), - stderr: "error: pathspec 'FETCH_HEAD' did not match".to_string(), - exit_code: Some(1), - termination: CommandTermination::Exited, - duration_ms: 1, - }; - - let local = crate::display_for_log(&sandbox.clone_failure_error( - failure.clone(), - "git checkout exact commit", - None, - CloneStep::Local, - )); - assert!(!local.contains("fabro install"), "{local}"); - assert!(local.contains("prepare the cloned repository"), "{local}"); - - let network = crate::display_for_log(&sandbox.clone_failure_error( - failure, - "git fetch exact commit", - None, - CloneStep::Network, - )); - assert!(network.contains("fabro install"), "{network}"); - } - - #[test] - fn clone_result_uses_stderr_before_stdout() { - let result = ExecResult { - stdout: "Could not resolve host: github.com".to_string(), - stderr: "fatal: destination path 'fabro' already exists".to_string(), - exit_code: Some(128), - termination: CommandTermination::Exited, - duration_ms: 1, - }; - - assert_eq!( - classify_docker_clone_result(&result, CredentialContext::FreshApp), - None - ); - } - - #[test] - fn container_config_has_no_bind_mounts_or_socket() { + fn driver_spec_maps_image_workspace_labels_env_and_limits() { + let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); let options = DockerSandboxOptions { env_vars: vec![ "FOO=bar".to_string(), @@ -2804,298 +253,56 @@ mod tests { ], memory_limit: Some(4_000_000_000), cpu_quota: Some(200_000), + network_mode: Some("none".to_string()), + auto_pull: false, ..DockerSandboxOptions::default() }; - let config = container_config(&options, None); - let host_config = config.host_config.expect("host config"); - assert!(host_config.binds.is_none()); - assert_eq!(host_config.memory, Some(4_000_000_000)); - assert_eq!(host_config.cpu_quota, Some(200_000)); - assert_eq!(config.working_dir.as_deref(), Some(WORKING_DIRECTORY)); - assert_eq!( - config.env, - Some(vec!["FOO=bar".to_string(), "BASH_ENV=".to_string()]) - ); - assert!( - config - .env - .unwrap() - .iter() - .all(|value| !value.starts_with("DOCKER_HOST=")) - ); - } + let spec = driver_spec(&options, Some(&run_id)); - #[test] - fn real_run_container_gets_name_and_labels() { - let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); + assert!(matches!( + &spec.source, + SandboxSource::Image { reference } if reference == "buildpack-deps:noble" + )); assert_eq!( - container_name(&run_id), - "fabro-run-01HY0000000000000000000000" + spec.name.as_deref(), + Some("fabro-run-01HY0000000000000000000000") ); - let labels = managed_labels::for_run(Some(&run_id)); - assert_eq!(labels.get(MANAGED_LABEL).map(String::as_str), Some("true")); + assert_eq!(spec.working_directory.as_deref(), Some(WORKING_DIRECTORY)); assert_eq!( - labels.get(RUN_ID_LABEL).map(String::as_str), + spec.labels.get(MANAGED_LABEL).map(String::as_str), + Some("true") + ); + assert_eq!( + spec.labels.get(RUN_ID_LABEL).map(String::as_str), Some("01HY0000000000000000000000") ); + assert_eq!(spec.env.get("FOO").map(String::as_str), Some("bar")); + // The driver blanks BASH_ENV on every container; a caller value is + // passed through here and overridden there. + assert_eq!(spec.resources.cpu_cores, Some(2)); + assert_eq!(spec.resources.memory_mb, Some(3815)); + assert!(matches!(spec.network, NetworkPolicy::Block)); + assert_eq!(spec.provider_config["auto_pull"], false); } #[test] - fn docker_access_command_uses_exec_in_workspace() { - assert_eq!( - docker_access_command( - "fabro-run-01HY0000000000000000000000", - "/workspace/rack-test" - ), - "docker exec -it fabro-run-01HY0000000000000000000000 sh -lc 'cd /workspace/rack-test && exec sh -l'" - ); + fn driver_spec_defaults_to_bridge_networking_without_a_name() { + let spec = driver_spec(&DockerSandboxOptions::default(), None); + assert!(spec.name.is_none()); + assert!(matches!(spec.network, NetworkPolicy::AllowAll)); + assert_eq!(spec.resources, Resources::default()); + assert!(!spec.labels.contains_key(RUN_ID_LABEL)); } #[test] - fn docker_access_command_quotes_container_identifier() { - assert_eq!( - docker_access_command("container with spaces", "/workspace/repo with spaces"), - "docker exec -it 'container with spaces' sh -lc \"cd '/workspace/repo with spaces' && exec sh -l\"" - ); - } - - #[test] - fn stdio_exec_options_attach_streams_without_tty() { - let (create, start) = docker_stdio_exec_options( - "python fake_agent.py".to_string(), - WORKING_DIRECTORY.to_string(), - Some(vec![ - "MODE=test".to_string(), - "BASH_ENV=/tmp/untrusted-startup".to_string(), - ]), - ); - - assert_eq!(create.attach_stdin, Some(true)); - assert_eq!(create.attach_stdout, Some(true)); - assert_eq!(create.attach_stderr, Some(true)); - assert_eq!(create.tty, Some(false)); - assert_eq!(create.working_dir.as_deref(), Some(WORKING_DIRECTORY)); - assert_eq!( - create.env, - Some(vec!["MODE=test".to_string(), "BASH_ENV=".to_string()]) - ); - assert_eq!( - create.cmd, - Some(vec![ - "/bin/bash".to_string(), - "-c".to_string(), - "python fake_agent.py".to_string() - ]) - ); - assert!(!start.detach); - assert!(!start.tty); - assert_eq!(start.output_capacity, None); - } - - #[tokio::test] - async fn controlled_shell_command_honors_stop_requested_before_pid_file_exists() { - let tempdir = tempfile::tempdir().expect("tempdir should be created"); - let stop_file = tempdir.path().join("stop"); - let pid_file = tempdir.path().join("pid"); - let block_fifo = tempdir.path().join("block"); - let stop_file = stop_file.to_string_lossy().into_owned(); - let pid_file = pid_file.to_string_lossy().into_owned(); - let block_fifo = block_fifo.to_string_lossy().into_owned(); - let marker = "fabro_controlled_shell_stop_sentinel"; - let command = docker_controlled_shell_command( - &format!( - "mkfifo {}; trap '' HUP TERM; read _ < {} # {marker}", - shell_quote(&block_fifo), - shell_quote(&block_fifo) - ), - &stop_file, - &pid_file, - ); - - fs::write(&stop_file, b"") - .await - .expect("early stop file should be written"); - let mut child = Command::new("/bin/bash"); - child.arg("-c").arg(command).kill_on_drop(true); - let output = if let Ok(output) = time::timeout(Duration::from_secs(5), child.output()).await - { - output.expect("controlled shell command should run") - } else { - kill_processes_with_marker(marker).await; - panic!("controlled shell command should honor an early stop request"); - }; - - assert!( - !output.status.success(), - "controlled shell command should be terminated by the stop request" - ); - let matching_processes = processes_with_marker(marker).await; - assert!( - matching_processes.is_empty(), - "controlled shell command should not leave child processes: {matching_processes:?}" - ); - } - - #[tokio::test] - async fn controlled_shell_command_preserves_stdin_for_user_command() { - let tempdir = tempfile::tempdir().expect("tempdir should be created"); - let stop_file = tempdir.path().join("stop"); - let pid_file = tempdir.path().join("pid"); - let stop_file = stop_file.to_string_lossy().into_owned(); - let pid_file = pid_file.to_string_lossy().into_owned(); - let command = docker_controlled_shell_command("cat", &stop_file, &pid_file); - - let mut child = Command::new("/bin/bash") - .arg("-c") - .arg(command) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .kill_on_drop(true) - .spawn() - .expect("controlled shell command should spawn"); - let mut stdin = child - .stdin - .take() - .expect("controlled shell command stdin should be piped"); - stdin - .write_all(b"abc\n") - .await - .expect("stdin should be written"); - drop(stdin); - - let output = time::timeout(Duration::from_secs(5), child.wait_with_output()) - .await - .expect("controlled shell command should not hang") - .expect("controlled shell command should run"); - - assert!( - output.status.success(), - "controlled shell command should exit successfully: {output:?}" - ); - assert_eq!(output.stdout, b"abc\n"); - } - - #[tokio::test] - async fn docker_stdio_process_state_does_not_cache_cancelled_on_stop_request() { - let state = DockerStdioProcessState::default(); - - assert!(state.request_stop_once().await); - assert_eq!(state.cached_termination().await, None); - assert!(!state.request_stop_once().await); - - let termination = StdioProcessTermination::exited(Some(143)); - state.cache_termination(termination).await; - assert_eq!(state.cached_termination().await, Some(termination)); - assert!(!state.request_stop_once().await); - } - - #[tokio::test] - async fn controlled_shell_command_skips_user_command_when_stop_already_requested() { - let tempdir = tempfile::tempdir().expect("tempdir should be created"); - let stop_file = tempdir.path().join("stop"); - let pid_file = tempdir.path().join("pid"); - let marker_file = tempdir.path().join("started"); - let stop_file = stop_file.to_string_lossy().into_owned(); - let pid_file = pid_file.to_string_lossy().into_owned(); - let marker_file = marker_file.to_string_lossy().into_owned(); - let command = docker_controlled_shell_command( - &format!("touch {}", shell_quote(&marker_file)), - &stop_file, - &pid_file, - ); - - fs::write(&stop_file, b"") - .await - .expect("early stop file should be written"); - let output = Command::new("/bin/bash") - .arg("-c") - .arg(command) - .output() - .await - .expect("controlled shell command should run"); - - assert!( - !output.status.success(), - "controlled shell command should exit as stopped" - ); - assert!( - !fs::try_exists(&marker_file) - .await - .expect("marker file existence should be checked"), - "controlled shell command should not start user command after an early stop" - ); - } - - async fn processes_with_marker(marker: &str) -> Vec { - let output = Command::new("ps") - .args(["-eo", "pid=,args="]) - .output() - .await - .expect("process probe should run"); - String::from_utf8_lossy(&output.stdout) - .lines() - .filter(|line| line.contains(marker)) - .map(str::to_string) - .collect() - } - - async fn kill_processes_with_marker(marker: &str) { - for line in processes_with_marker(marker).await { - if let Some(pid) = line.split_whitespace().next() { - let _ = Command::new("kill").args(["-KILL", pid]).status().await; - } - } - } - - #[test] - fn label_validation_rejects_unmanaged_container() { - let labels = HashMap::new(); - let error = verify_managed_labels("abc", &labels, None).unwrap_err(); - assert!( - error - .to_string() - .contains("missing label sh.fabro.managed=true") - ); - } - - #[test] - fn single_file_tar_contains_named_file() { - let bytes = build_single_file_tar("nested.txt", b"hello", 0o644).unwrap(); - let mut archive = tar::Archive::new(Cursor::new(bytes)); - let mut entries = archive.entries().unwrap(); - let mut entry = entries.next().unwrap().unwrap(); - assert_eq!(entry.path().unwrap().to_string_lossy(), "nested.txt"); - assert_eq!(entry.header().mode().unwrap(), 0o644); - let mut content = String::new(); - entry.read_to_string(&mut content).unwrap(); - assert_eq!(content, "hello"); - } - - #[test] - fn single_file_tar_applies_private_mode() { - let bytes = build_single_file_tar("blob.json", b"{}", 0o600).unwrap(); - let mut archive = tar::Archive::new(Cursor::new(bytes)); - let mut entries = archive.entries().unwrap(); - let entry = entries.next().unwrap().unwrap(); - assert_eq!(entry.header().mode().unwrap(), 0o600); - } - - fn test_docker_sandbox(docker: Docker, container_id: &str) -> DockerSandbox { - let sandbox = DockerSandbox::with_docker_client( - docker, - DockerSandboxOptions::default(), - None, - None, - None, - None, - None, - None, - ) - .expect("test sandbox should build"); - sandbox - .container_id - .set(container_id.to_string()) - .expect("test container should initialize once"); - sandbox + fn managed_label_check_requires_fabro_ownership_and_matching_run() { + let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); + let mut labels = BTreeMap::new(); + assert!(verify_managed_labels("c1", &labels, None).is_err()); + labels.insert(MANAGED_LABEL.to_string(), "true".to_string()); + assert!(verify_managed_labels("c1", &labels, None).is_ok()); + assert!(verify_managed_labels("c1", &labels, Some(&run_id)).is_err()); + labels.insert(RUN_ID_LABEL.to_string(), run_id.to_string()); + assert!(verify_managed_labels("c1", &labels, Some(&run_id)).is_ok()); } } diff --git a/lib/components/fabro-sandbox/src/driver_sandbox.rs b/lib/components/fabro-sandbox/src/driver_sandbox.rs index 71d222d76..27fb460ba 100644 --- a/lib/components/fabro-sandbox/src/driver_sandbox.rs +++ b/lib/components/fabro-sandbox/src/driver_sandbox.rs @@ -13,20 +13,28 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::sync::{Arc, OnceLock}; +use std::sync::{Arc, Mutex, OnceLock, PoisonError}; use std::time::{Duration, Instant}; use async_trait::async_trait; +use fabro_github::GitHubCredentials; +use fabro_github::token_source::InstallationTokenSource; use fabro_types::SandboxProviderKind; use sandbox_driver::{ - FileKind, LifecycleTimers, Sandbox as DriverHandle, SandboxProvider as _, SandboxSource, + Action, Event, EventBody, EventContext, EventObserver, FileKind, LifecycleTimers, ProgressCode, + PtyOptions, PtySize, Sandbox as DriverHandle, SandboxProvider as DriverProvider, SandboxSource, SandboxSpec as DriverSpec, SandboxState, Search as _, WaitOptions, }; use sandbox_driver_host::HostProvider; use tokio::fs; +use tokio::sync::OnceCell; use tokio_util::sync::CancellationToken; -use crate::RetryPlan; +use crate::clone::{self, GitHubClone}; +use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason}; +use crate::push_credentials::{self, PushCredentialState}; +use crate::terminal::{DriverTerminalSession, TerminalSize}; +use crate::{GitRunInfo, GitSetupIntent, RefreshOutcome, RetryPlan}; /// A sandbox on the worker host at `working_directory`, the fabro `local` /// kind, served by the driver's in-process Host provider. @@ -59,10 +67,158 @@ use crate::sandbox::{ WalkOptions, }; +/// Where a clone-based provider puts its files: the run works under +/// `workspace_root`, and repositories check out under `repos_root`. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct WorkspaceLayout { + pub(crate) workspace_root: String, + pub(crate) repos_root: String, +} + +/// What `initialize` does to the workspace once the sandbox runs. +enum WorkspacePlan { + /// Clone this GitHub repository into the layout. + Clone(GitHubClone), + /// Create the empty workspace root and nothing else. + Empty(EmptyWorkspaceReason), + /// The workspace was prepared by an earlier process; leave it alone. + Attached, +} + +/// Fabro's clone-based workspace on an isolated sandbox: the layout, the +/// clone it performs, and the GitHub credentials its checkout carries. +pub(crate) struct RepoWorkspace { + layout: WorkspaceLayout, + plan: WorkspacePlan, + credentials: PushCredentialState, + repo_cloned: OnceLock, + origin_url: OnceLock, + /// The directory the run works in once known: the repository link for a + /// clone, the workspace root otherwise. + execution_directory: OnceLock, + /// The real checkout behind the workspace link, for traversals that + /// must not start at a symlink. + checkout_path: OnceLock, +} + +impl RepoWorkspace { + /// Decide the clone for a new sandbox. Fails before any provider call + /// when the selectors are inconsistent (a pin without a branch, a + /// non-GitHub origin without `skip_clone`). + #[expect( + clippy::too_many_arguments, + reason = "the clone selectors are validated together by decide_clone" + )] + pub(crate) fn plan( + layout: WorkspaceLayout, + skip_clone: bool, + clone_origin_url: Option<&str>, + clone_branch: Option<&str>, + clone_tag: Option<&str>, + clone_commit_sha: Option<&str>, + clone_depth: Option, + github_app: Option<&GitHubCredentials>, + ) -> crate::Result { + let decision = clone_source::decide_clone( + skip_clone, + clone_origin_url, + clone_branch, + clone_tag, + clone_commit_sha, + )?; + let credentials = PushCredentialState::new(push_credentials::build_token_source( + github_app, + clone_origin_url, + )?); + let plan = match decision { + CloneDecision::EmptyWorkspace { reason } => WorkspacePlan::Empty(reason), + CloneDecision::GitHub { + origin_url, + branch, + tag, + commit_sha, + } => WorkspacePlan::Clone(GitHubClone { + origin_url, + branch, + tag, + commit_sha, + depth: clone_depth, + }), + }; + Ok(Self { + layout, + plan, + credentials, + repo_cloned: OnceLock::new(), + origin_url: OnceLock::new(), + execution_directory: OnceLock::new(), + checkout_path: OnceLock::new(), + }) + } + + /// A workspace prepared by an earlier process, described by the run + /// record. Pushes from a reattached sandbox use whatever credentials the + /// checkout's `origin` already carries. + pub(crate) fn attached( + layout: WorkspaceLayout, + repo_cloned: bool, + working_directory: String, + clone_origin_url: Option, + ) -> Self { + let workspace = Self { + layout, + plan: WorkspacePlan::Attached, + credentials: PushCredentialState::new(None), + repo_cloned: OnceLock::new(), + origin_url: OnceLock::new(), + execution_directory: OnceLock::new(), + checkout_path: OnceLock::new(), + }; + let _ = workspace.repo_cloned.set(repo_cloned); + let _ = workspace.execution_directory.set(working_directory); + if repo_cloned { + if let Some(origin) = clone_origin_url { + if let Ok(repo_layout) = clone_source::github_repo_layout( + &origin, + &workspace.layout.workspace_root, + &workspace.layout.repos_root, + ) { + let _ = workspace.checkout_path.set(repo_layout.primary_repo_path); + } + let _ = workspace.origin_url.set(origin); + } + } + workspace + } + + fn repo_cloned(&self) -> bool { + self.repo_cloned.get().copied().unwrap_or(false) + } + + fn working_directory(&self) -> &str { + self.execution_directory + .get() + .map_or(self.layout.workspace_root.as_str(), String::as_str) + } +} + +/// A sandbox that does not exist yet: `initialize` creates it from the +/// spec on the provider. +struct PendingCreate { + provider: Arc, + spec: DriverSpec, + /// The image or snapshot named by the spec, for pull progress events. + source: Option, +} + /// A fabro sandbox backed by a sandbox-driver handle. pub struct DriverSandbox { kind: SandboxProviderKind, - handle: Arc, + /// Set at construction for an existing sandbox, at `initialize` for a + /// pending one. + handle: OnceCell>, + pending: Option, + workspace: Option, env_policy: ExplicitEnvPolicy, event_callback: Option, /// `(platform, os_version)` learned from the sandbox at initialize or @@ -76,6 +232,43 @@ impl DriverSandbox { /// is isolated and takes the caller's environment as composed. #[must_use] pub fn new(kind: SandboxProviderKind, handle: Arc) -> Self { + let sandbox = Self::empty(kind); + let _ = sandbox.handle.set(handle); + sandbox + } + + /// A sandbox `initialize` will create from `spec` on `provider`, then + /// prepare per `workspace`. + pub(crate) fn pending( + kind: SandboxProviderKind, + provider: Arc, + spec: DriverSpec, + source: Option, + workspace: RepoWorkspace, + ) -> Self { + let mut sandbox = Self::empty(kind); + sandbox.pending = Some(PendingCreate { + provider, + spec, + source, + }); + sandbox.workspace = Some(workspace); + sandbox + } + + /// An existing sandbox reattached by handle, with the workspace an + /// earlier process prepared. + pub(crate) fn attached( + kind: SandboxProviderKind, + handle: Arc, + workspace: RepoWorkspace, + ) -> Self { + let mut sandbox = Self::new(kind, handle); + sandbox.workspace = Some(workspace); + sandbox + } + + fn empty(kind: SandboxProviderKind) -> Self { let env_policy = if kind.is_local() { ExplicitEnvPolicy::FilterSensitive } else { @@ -83,7 +276,9 @@ impl DriverSandbox { }; Self { kind, - handle, + handle: OnceCell::new(), + pending: None, + workspace: None, env_policy, event_callback: None, platform: OnceLock::new(), @@ -101,14 +296,37 @@ impl DriverSandbox { } /// The driver handle, for callers that need a facet fabro's trait does - /// not carry (git, services, access). - #[must_use] - pub fn handle(&self) -> &Arc { - &self.handle + /// not carry (git, services, access). Absent until a pending sandbox is + /// initialized. + pub fn handle(&self) -> crate::Result<&Arc> { + self.handle.get().ok_or_else(|| { + crate::Error::message(format!( + "{} sandbox is not initialized; call initialize() first", + self.kind + )) + }) } - fn exec(&self) -> SandboxExec<'_> { - SandboxExec::new(self.handle.exec(), self.env_policy) + fn exec(&self) -> crate::Result> { + let mut exec = SandboxExec::new(self.handle()?.exec(), self.env_policy); + if let Some(workspace) = &self.workspace { + if let Some(dir) = workspace.execution_directory.get() { + exec = exec.with_working_dir(dir.clone()); + } + } + Ok(exec) + } + + /// Resolve a caller path against fabro's working directory. The driver + /// resolves relative paths against the sandbox's own working directory, + /// which sits above a cloned repository's link. + fn resolve(&self, path: &str) -> String { + match &self.workspace { + Some(workspace) if workspace.execution_directory.get().is_some() => { + sandbox::resolve_path(path, workspace.working_directory()) + } + _ => path.to_string(), + } } fn provider_name(&self) -> String { @@ -123,7 +341,7 @@ impl DriverSandbox { } fn search(&self) -> crate::Result> { - self.handle.search().ok_or_else(|| { + self.handle()?.search().ok_or_else(|| { crate::Error::message(format!( "sandbox provider `{}` does not support search", self.kind @@ -131,18 +349,142 @@ impl DriverSandbox { }) } + /// Create the sandbox on the provider when it does not exist yet. + async fn ensure_created(&self) -> crate::Result<()> { + if self.handle.get().is_some() { + return Ok(()); + } + let Some(pending) = &self.pending else { + return self.handle().map(|_| ()); + }; + let observer = Arc::new(CreateProgress::new( + pending.source.clone(), + self.event_callback.clone(), + )); + let handle = pending + .provider + .create(&pending.spec, Some(EventContext::new(observer))) + .await + .map_err(|error| { + crate::Error::context(format!("Failed to create {} sandbox", self.kind), error) + })?; + let _ = self.handle.set(handle); + Ok(()) + } + /// Bring the sandbox to `Running` with a verified Bash, and learn its /// platform. Shared by initialize and start. async fn make_ready(&self) -> crate::Result<()> { - sandbox_driver::activate(self.handle.as_ref(), &WaitOptions::default()).await?; + sandbox_driver::activate(self.handle()?.as_ref(), &WaitOptions::default()).await?; self.learn_platform().await } + /// Prepare the workspace after the sandbox runs for the first time: + /// an empty root, or fabro's clone. + async fn prepare_workspace(&self) -> crate::Result<()> { + let Some(workspace) = &self.workspace else { + return Ok(()); + }; + match &workspace.plan { + WorkspacePlan::Attached => Ok(()), + WorkspacePlan::Empty(reason) => { + if matches!(reason, EmptyWorkspaceReason::MissingOrigin) { + tracing::warn!( + provider = %self.kind, + reason = reason.message(), + "Clone source missing for clone-based sandbox" + ); + } + self.handle()? + .fs() + .create_dir(&workspace.layout.workspace_root) + .await + .map_err(|error| { + crate::Error::context( + format!("Failed to create {}", workspace.layout.workspace_root), + error, + ) + })?; + let _ = workspace.repo_cloned.set(false); + Ok(()) + } + WorkspacePlan::Clone(plan) => { + self.emit(SandboxEvent::GitCloneStarted { + url: plan.origin_url.clone(), + branch: plan.branch.clone(), + }); + let started = Instant::now(); + let handle = self.handle()?; + // The clone names every directory it touches, so it runs + // without fabro's working-directory override. + let exec = SandboxExec::new(handle.exec(), self.env_policy); + let outcome = clone::clone_github_repo( + &self.kind, + handle.as_ref(), + &exec, + plan, + &workspace.layout.workspace_root, + &workspace.layout.repos_root, + &workspace.credentials, + ) + .await; + match outcome { + Ok(outcome) => { + let _ = workspace.repo_cloned.set(true); + let _ = workspace.origin_url.set(plan.origin_url.clone()); + let _ = workspace + .checkout_path + .set(outcome.layout.primary_repo_path.clone()); + let _ = workspace + .execution_directory + .set(outcome.layout.execution_directory.clone()); + self.emit(SandboxEvent::GitCloneCompleted { + url: plan.origin_url.clone(), + duration_ms: elapsed_ms(started), + }); + Ok(()) + } + Err(error) => { + self.emit(SandboxEvent::GitCloneFailed { + url: plan.origin_url.clone(), + error: error.to_string(), + causes: error.causes(), + }); + Err(error) + } + } + } + } + } + + /// Open an interactive shell in the sandbox's working directory over the + /// driver's Pty facet. + pub async fn open_terminal(&self, size: TerminalSize) -> crate::Result { + let handle = self.handle()?; + let pty = handle.pty().ok_or_else(|| { + crate::Error::message(format!( + "sandbox provider `{}` does not support terminals", + self.kind + )) + })?; + let mut options = PtyOptions::default(); + options.size = PtySize { + rows: size.rows, + cols: size.cols, + }; + options.working_dir = Some(self.working_directory().to_string()); + let session = pty + .open(&options) + .await + .map_err(|error| crate::Error::context("Failed to open sandbox terminal", error))?; + Ok(DriverTerminalSession::new(session)) + } + /// Ask the sandbox for its platform once; `platform` and `os_version` /// report `unknown` until this has run. async fn learn_platform(&self) -> crate::Result<()> { if self.platform.get().is_none() { - let info = self.handle.platform_info().await?; + let info = self.handle()?.platform_info().await?; let platform = fabro_platform_name(&info.os).to_string(); let os_version = if info.version.is_empty() { platform.clone() @@ -160,13 +502,109 @@ impl DriverSandbox { /// walked as given. fn walk_base(&self, base: &str, relative_start: &str) -> String { if base == self.working_directory() || base.is_empty() || base == "." { + // A cloned repository is reached through a workspace link. The + // driver refuses a symlinked traversal root, so walk the real + // checkout; results are reported under the link. + if let Some(checkout) = self + .workspace + .as_ref() + .and_then(|workspace| workspace.checkout_path.get()) + { + return sandbox::join_sandbox_path(checkout, relative_start); + } if relative_start.is_empty() { ".".to_string() } else { relative_start.to_string() } } else { - sandbox::join_sandbox_path(base, relative_start) + sandbox::join_sandbox_path(&self.resolve(base), relative_start) + } + } +} + +/// Turns the driver's create-time progress into fabro's snapshot events: +/// an image pull starts `SnapshotPulling` and the create's completion ends +/// it. A create without a pull emits nothing. +struct CreateProgress { + source: Option, + callback: Option, + pull_started: Mutex>, +} + +impl CreateProgress { + fn new(source: Option, callback: Option) -> Self { + Self { + source, + callback, + pull_started: Mutex::new(None), + } + } + + fn emit(&self, event: SandboxEvent) { + event.trace(); + if let Some(cb) = &self.callback { + cb(event); + } + } + + fn name(&self) -> String { + self.source.clone().unwrap_or_default() + } +} + +#[async_trait] +impl EventObserver for CreateProgress { + async fn observe(&self, event: Event) { + match &event.body { + EventBody::OperationProgress { progress, .. } + if progress.code.as_str() == ProgressCode::IMAGE_PULL => + { + let mut started = self + .pull_started + .lock() + .unwrap_or_else(PoisonError::into_inner); + if started.is_none() { + *started = Some(Instant::now()); + drop(started); + self.emit(SandboxEvent::SnapshotPulling { name: self.name() }); + } + } + EventBody::OperationCompleted { + action: Action::Create, + .. + } => { + let started = self + .pull_started + .lock() + .unwrap_or_else(PoisonError::into_inner) + .take(); + if let Some(started) = started { + self.emit(SandboxEvent::SnapshotReady { + name: self.name(), + duration_ms: elapsed_ms(started), + }); + } + } + EventBody::OperationFailed { + action: Action::Create, + error, + .. + } => { + let started = self + .pull_started + .lock() + .unwrap_or_else(PoisonError::into_inner) + .take(); + if started.is_some() { + self.emit(SandboxEvent::SnapshotFailed { + name: self.name(), + error: error.message.clone(), + causes: error.causes.clone(), + }); + } + } + _ => {} } } } @@ -186,17 +624,17 @@ fn file_context(action: &str, path: &str) -> String { #[async_trait] impl Sandbox for DriverSandbox { async fn read_file_bytes(&self, path: &str) -> crate::Result> { - self.handle + self.handle()? .fs() - .read(path) + .read(&self.resolve(path)) .await .map_err(|error| crate::Error::context(file_context("read", path), error)) } async fn write_file(&self, path: &str, content: &str) -> crate::Result<()> { - self.handle + self.handle()? .fs() - .write(path, content.as_bytes()) + .write(&self.resolve(path), content.as_bytes()) .await .map_err(|error| crate::Error::context(file_context("write", path), error)) } @@ -210,17 +648,17 @@ impl Sandbox for DriverSandbox { file_context("delete", path) ))); } - self.handle + self.handle()? .fs() - .delete(path, false) + .delete(&self.resolve(path), false) .await .map_err(|error| crate::Error::context(file_context("delete", path), error)) } async fn file_exists(&self, path: &str) -> crate::Result { - self.handle + self.handle()? .fs() - .exists(path) + .exists(&self.resolve(path)) .await .map_err(|error| crate::Error::context(file_context("stat", path), error)) } @@ -231,9 +669,9 @@ impl Sandbox for DriverSandbox { depth: Option, ) -> crate::Result> { let entries = self - .handle + .handle()? .fs() - .list_dir(path, depth.unwrap_or(1)) + .list_dir(&self.resolve(path), depth.unwrap_or(1)) .await .map_err(|error| crate::Error::context(file_context("list", path), error))?; let mut entries: Vec = entries @@ -258,7 +696,7 @@ impl Sandbox for DriverSandbox { env_vars: Option<&HashMap>, cancel_token: Option, ) -> crate::Result { - self.exec() + self.exec()? .run( command, Some(Duration::from_millis(timeout_ms)), @@ -273,7 +711,7 @@ impl Sandbox for DriverSandbox { &self, request: ExecStreamingRequest<'_>, ) -> crate::Result { - self.exec().run_streaming(request).await + self.exec()?.run_streaming(request).await } async fn spawn_stdio_process( @@ -283,7 +721,7 @@ impl Sandbox for DriverSandbox { env_vars: Option<&HashMap>, cancel_token: Option, ) -> crate::Result { - self.exec() + self.exec()? .spawn_stdio(command, working_dir, env_vars, cancel_token) .await } @@ -300,7 +738,7 @@ impl Sandbox for DriverSandbox { driver_options.include.clone_from(&options.glob_filter); let matches = self .search()? - .grep(pattern, path, &driver_options) + .grep(pattern, &self.resolve(path), &driver_options) .await .map_err(|error| crate::Error::context("Failed to search file contents", error))?; Ok(matches @@ -337,7 +775,7 @@ impl Sandbox for DriverSandbox { let size = match file.size { Some(size) => size, None => { - self.handle + self.handle()? .fs() .metadata(&path) .await @@ -359,9 +797,9 @@ impl Sandbox for DriverSandbox { remote_path: &str, local_path: &Path, ) -> crate::Result<()> { - self.handle + self.handle()? .fs() - .download(remote_path, local_path) + .download(&self.resolve(remote_path), local_path) .await .map_err(|error| crate::Error::context(file_context("download", remote_path), error)) } @@ -371,19 +809,26 @@ impl Sandbox for DriverSandbox { local_path: &Path, remote_path: &str, ) -> crate::Result<()> { - self.handle + self.handle()? .fs() - .upload(local_path, remote_path) + .upload(local_path, &self.resolve(remote_path)) .await .map_err(|error| crate::Error::context(file_context("upload", remote_path), error)) } + /// Create the sandbox when it is pending, bring it to `Running`, and + /// prepare fabro's workspace (empty root or clone) on first use. async fn initialize(&self) -> crate::Result<()> { self.emit(SandboxEvent::Initializing { provider: self.provider_name(), }); let started = Instant::now(); - let result = self.make_ready().await; + let result = async { + self.ensure_created().await?; + self.make_ready().await?; + self.prepare_workspace().await + } + .await; let duration_ms = elapsed_ms(started); match &result { Ok(()) => self.emit(SandboxEvent::Ready { @@ -407,7 +852,7 @@ impl Sandbox for DriverSandbox { /// Idempotent access-time check: a running sandbox is left alone; a /// stopped or paused one is brought back and its Bash verified. async fn activate(&self) -> crate::Result<()> { - let status = self.handle.describe().await?; + let status = self.handle()?.describe().await?; if status.state == SandboxState::Running { return Ok(()); } @@ -439,7 +884,10 @@ impl Sandbox for DriverSandbox { provider: self.provider_name(), }); let started = Instant::now(); - let result = self.handle.stop().await.map_err(crate::Error::from); + let result = match self.handle() { + Ok(handle) => handle.stop().await.map_err(crate::Error::from), + Err(error) => Err(error), + }; match &result { Ok(()) => self.emit(SandboxEvent::StopCompleted { provider: self.provider_name(), @@ -459,7 +907,7 @@ impl Sandbox for DriverSandbox { provider: self.provider_name(), }); let started = Instant::now(); - let result = self.handle.delete().await.map_err(crate::Error::from); + let result = self.release().await; match &result { Ok(()) => self.emit(SandboxEvent::DeleteCompleted { provider: self.provider_name(), @@ -482,7 +930,7 @@ impl Sandbox for DriverSandbox { provider: self.provider_name(), }); let started = Instant::now(); - let result = self.handle.delete().await.map_err(crate::Error::from); + let result = self.release().await; match &result { Ok(()) => self.emit(SandboxEvent::CleanupCompleted { provider: self.provider_name(), @@ -497,12 +945,21 @@ impl Sandbox for DriverSandbox { result } + /// The directory the run works in: the cloned repository's link for a + /// clone-based workspace, the provider's working directory otherwise. fn working_directory(&self) -> &str { - self.handle.working_directory() + if let Some(workspace) = &self.workspace { + return workspace.working_directory(); + } + self.handle + .get() + .map_or("", |handle| handle.working_directory()) } fn runtime_directory(&self) -> Option<&str> { - self.handle.runtime_directory() + self.handle + .get() + .and_then(|handle| handle.runtime_directory()) } fn platform(&self) -> &str { @@ -521,12 +978,15 @@ impl Sandbox for DriverSandbox { /// The provider's id for this sandbox, or empty for `local`: a local /// sandbox is its working directory, which the run record already /// carries, and its Host registry id does not outlive the process. + /// Empty for a pending sandbox that has not been created. fn sandbox_info(&self) -> String { if self.kind.is_local() { - String::new() - } else { - self.handle.id().to_string() + return String::new(); } + self.handle + .get() + .map(|handle| handle.id().to_string()) + .unwrap_or_default() } async fn set_autostop_interval(&self, minutes: i32) -> crate::Result<()> { @@ -535,7 +995,7 @@ impl Sandbox for DriverSandbox { .ok() .filter(|minutes| *minutes > 0) .map(Duration::from_mins); - match self.handle.set_timers(&timers).await { + match self.handle()?.set_timers(&timers).await { // A provider without timers has nothing to stop automatically. Ok(()) | Err(sandbox_driver::Error::Unsupported { .. }) => Ok(()), Err(error) => Err(crate::Error::context( @@ -545,33 +1005,108 @@ impl Sandbox for DriverSandbox { } } + async fn setup_git(&self, intent: &GitSetupIntent) -> crate::Result> { + if !self.repo_cloned() { + return Ok(None); + } + sandbox::setup_git_via_exec(self, intent).await.map(Some) + } + + fn resume_setup_commands(&self, run_branch: &str) -> Vec { + if !self.repo_cloned() { + return Vec::new(); + } + vec![format!( + "git fetch origin {} && git checkout {}", + sandbox::shell_quote(run_branch), + sandbox::shell_quote(run_branch) + )] + } + async fn git_push_ref(&self, refspec: &str, plan: &RetryPlan) -> Result { - let has_origin = match self - .exec_command("git remote get-url origin", 10_000, None, None, None) - .await - { - Ok(result) if result.is_success() => true, - Ok(_) => false, - Err(err) => { - return Err(PushError { - report: PushReport::default(), - error: crate::Error::context("git remote get-url origin", err), - }); + let Some(workspace) = &self.workspace else { + // A designated directory: push only when the checkout has an + // origin, with whatever credentials its URL already carries. + let has_origin = match self + .exec_command("git remote get-url origin", 10_000, None, None, None) + .await + { + Ok(result) if result.is_success() => true, + Ok(_) => false, + Err(err) => { + return Err(PushError { + report: PushReport::default(), + error: crate::Error::context("git remote get-url origin", err), + }); + } + }; + if !has_origin { + return Ok(PushReport::default()); } + return sandbox::git_push_via_exec(self, None, refspec, plan).await; }; - if !has_origin { + if !workspace.repo_cloned() { return Ok(PushReport::default()); } - // No managed credentials yet: the checkout pushes with whatever the - // remote URL already carries. - sandbox::git_push_via_exec(self, None, refspec, plan).await + let credentials = workspace + .origin_url + .get() + .map(|origin_url| (&workspace.credentials, origin_url.as_str())); + sandbox::git_push_via_exec(self, credentials, refspec, plan).await + } + + fn origin_url(&self) -> Option<&str> { + let workspace = self.workspace.as_ref()?; + if !workspace.repo_cloned() { + return None; + } + workspace.origin_url.get().map(String::as_str) + } + + #[tracing::instrument(name = "git_op", skip_all, fields(op = "refresh-credentials"))] + async fn refresh_push_credentials(&self) -> crate::Result { + let Some(workspace) = &self.workspace else { + return Ok(RefreshOutcome::none()); + }; + if !workspace.repo_cloned() { + return Ok(RefreshOutcome::none()); + } + let Some(origin_url) = workspace.origin_url.get() else { + return Ok(RefreshOutcome::none()); + }; + workspace + .credentials + .refresh(origin_url, |auth_url| { + push_credentials::set_auth_url_via_exec(self, auth_url) + }) + .await + } + + fn push_token_source(&self) -> Option> { + self.workspace + .as_ref() + .and_then(|workspace| workspace.credentials.source().cloned()) + } + + /// The local command that opens a shell in the sandbox, from the + /// provider's access facet. `None` when the provider has no such + /// command (the local sandbox is the host). + async fn ssh_access_command(&self) -> crate::Result> { + let Some(shell) = self.handle()?.shell_command() else { + return Ok(None); + }; + shell + .shell_command() + .await + .map(Some) + .map_err(|error| crate::Error::context("Failed to build sandbox shell command", error)) } async fn get_preview_url( &self, port: u16, ) -> crate::Result)>> { - let Some(previews) = self.handle.preview_urls() else { + let Some(previews) = self.handle()?.preview_urls() else { return Ok(None); }; let preview = previews @@ -585,14 +1120,30 @@ impl Sandbox for DriverSandbox { } } +impl DriverSandbox { + fn repo_cloned(&self) -> bool { + self.workspace + .as_ref() + .is_some_and(RepoWorkspace::repo_cloned) + } + + /// Delete the sandbox on the provider. A pending sandbox that was never + /// created has nothing to release. + async fn release(&self) -> crate::Result<()> { + match self.handle.get() { + Some(handle) => handle.delete().await.map_err(crate::Error::from), + None if self.pending.is_some() => Ok(()), + None => self.handle().map(|_| ()), + } + } +} + fn elapsed_ms(started: Instant) -> u64 { u64::try_from(started.elapsed().as_millis()).unwrap_or(u64::MAX) } #[cfg(test)] mod tests { - use std::sync::Mutex; - use fabro_types::CommandTermination; use sandbox_driver::{SandboxProvider as _, SandboxSource, SandboxSpec}; use sandbox_driver_host::HostProvider; @@ -832,9 +1383,9 @@ mod tests { "", "local sandboxes are identified by directory" ); - let isolated = - DriverSandbox::new(SandboxProviderKind::DOCKER, Arc::clone(f.sandbox.handle())); - assert_eq!(isolated.sandbox_info(), f.sandbox.handle().id().to_string()); + let handle = Arc::clone(f.sandbox.handle().unwrap()); + let isolated = DriverSandbox::new(SandboxProviderKind::DOCKER, Arc::clone(&handle)); + assert_eq!(isolated.sandbox_info(), handle.id().to_string()); f.sandbox.stop().await.unwrap(); f.sandbox.activate().await.unwrap(); diff --git a/lib/components/fabro-sandbox/src/error.rs b/lib/components/fabro-sandbox/src/error.rs index bf0d678fc..f63dc5d49 100644 --- a/lib/components/fabro-sandbox/src/error.rs +++ b/lib/components/fabro-sandbox/src/error.rs @@ -1,7 +1,5 @@ use std::fmt::Write as _; -#[cfg(feature = "docker")] -use bollard::errors::Error as BollardError; use fabro_util::error::{collect_causes, render_with_causes}; use crate::ExecResult; @@ -26,29 +24,6 @@ pub enum Error { source: anyhow::Error, }, - #[cfg(feature = "docker")] - #[error("Failed to connect to Docker daemon")] - DockerConnect { - #[source] - source: BollardError, - }, - - #[cfg(feature = "docker")] - #[error("Failed to inspect Docker image {image}")] - DockerImageInspect { - image: String, - #[source] - source: BollardError, - }, - - #[cfg(feature = "docker")] - #[error("Failed to pull Docker image {image}")] - DockerImagePull { - image: String, - #[source] - source: BollardError, - }, - /// A sandbox-driver failure: provider, transport, or an operation whose /// outcome is unknown. The driver's own variants stay reachable through /// [`Error::driver`] so callers can act on `NotFound`, `Unsupported`, @@ -101,27 +76,6 @@ impl Error { default_redacted_output_tail(self) } - #[cfg(feature = "docker")] - pub fn docker_connect(source: BollardError) -> Self { - Self::DockerConnect { source } - } - - #[cfg(feature = "docker")] - pub fn docker_image_inspect(image: impl Into, source: BollardError) -> Self { - Self::DockerImageInspect { - image: image.into(), - source, - } - } - - #[cfg(feature = "docker")] - pub fn docker_image_pull(image: impl Into, source: BollardError) -> Self { - Self::DockerImagePull { - image: image.into(), - source, - } - } - pub fn causes(&self) -> Vec { collect_causes(self) } diff --git a/lib/components/fabro-sandbox/src/exec.rs b/lib/components/fabro-sandbox/src/exec.rs index 64931e90b..688aecd2c 100644 --- a/lib/components/fabro-sandbox/src/exec.rs +++ b/lib/components/fabro-sandbox/src/exec.rs @@ -89,9 +89,12 @@ pub fn is_sensitive_env_var(key: &str) -> bool { /// Fabro's exec policy bound to one driver [`Exec`] facet. pub struct SandboxExec<'a> { - exec: &'a dyn Exec, - env_policy: ExplicitEnvPolicy, - stop_grace: Duration, + exec: &'a dyn Exec, + env_policy: ExplicitEnvPolicy, + stop_grace: Duration, + /// Where a command runs when the caller names no directory. `None` + /// leaves the choice to the provider's own working directory. + working_dir: Option, } impl<'a> SandboxExec<'a> { @@ -101,9 +104,19 @@ impl<'a> SandboxExec<'a> { exec, env_policy, stop_grace: DEFAULT_STOP_GRACE, + working_dir: None, } } + /// The directory commands run in when the caller names none. Fabro's + /// working directory can sit below the provider's (a cloned repository + /// inside the container workspace), so it is passed explicitly. + #[must_use] + pub fn with_working_dir(mut self, working_dir: impl Into) -> Self { + self.working_dir = Some(working_dir.into()); + self + } + /// Time between `TERM` and `KILL` when a command is stopped. #[must_use] pub fn with_stop_grace(mut self, stop_grace: Duration) -> Self { @@ -161,7 +174,7 @@ impl<'a> SandboxExec<'a> { let started = Instant::now(); let mut spec = ExecSpec::bash(command).no_timeout(); - if let Some(dir) = working_dir { + if let Some(dir) = working_dir.or(self.working_dir.as_deref()) { spec = spec.working_dir(dir); } for (key, value) in self.explicit_env(env_vars) { @@ -222,7 +235,7 @@ impl<'a> SandboxExec<'a> { cancel_token: Option, ) -> crate::Result { let mut spec = SpawnSpec::bash(format!("exec {command}")); - if let Some(dir) = working_dir { + if let Some(dir) = working_dir.or(self.working_dir.as_deref()) { spec = spec.working_dir(dir); } for (key, value) in self.explicit_env(env_vars) { diff --git a/lib/components/fabro-sandbox/src/from_environment.rs b/lib/components/fabro-sandbox/src/from_environment.rs index 8a392b2f9..820e451ec 100644 --- a/lib/components/fabro-sandbox/src/from_environment.rs +++ b/lib/components/fabro-sandbox/src/from_environment.rs @@ -5,7 +5,6 @@ use std::path::{Path, PathBuf}; -#[cfg(feature = "docker")] use fabro_types::settings::ResolveError; #[cfg(feature = "daytona")] use fabro_types::settings::run::DockerfileSource as ResolvedDockerfileSource; @@ -20,7 +19,6 @@ use crate::config::{ }; #[cfg(feature = "daytona")] use crate::daytona::DaytonaConfig; -#[cfg(feature = "docker")] use crate::docker::DockerSandboxOptions; #[cfg(feature = "daytona")] @@ -74,7 +72,6 @@ pub fn daytona_config_from_environment( } } -#[cfg(feature = "docker")] #[must_use] pub fn docker_config_from_environment( settings: &RunEnvironmentSettings, @@ -96,7 +93,6 @@ pub fn docker_config_from_environment( docker_config_from_environment_env(settings, clone, env) } -#[cfg(feature = "docker")] pub fn docker_config_from_environment_with_secrets( settings: &RunEnvironmentSettings, clone: &RunCloneSettings, @@ -106,7 +102,6 @@ pub fn docker_config_from_environment_with_secrets( Ok(docker_config_from_environment_env(settings, clone, env)) } -#[cfg(feature = "docker")] fn docker_config_from_environment_env( settings: &RunEnvironmentSettings, clone: &RunCloneSettings, diff --git a/lib/components/fabro-sandbox/src/git_retry.rs b/lib/components/fabro-sandbox/src/git_retry.rs index fbd805c0d..288f5c9fe 100644 --- a/lib/components/fabro-sandbox/src/git_retry.rs +++ b/lib/components/fabro-sandbox/src/git_retry.rs @@ -194,6 +194,42 @@ pub fn classify_failure(message: &str, cred: CredentialContext) -> Option Option { + match error { + sandbox_driver::Error::Exec(failure) => classify_output( + &String::from_utf8_lossy(failure.stderr()), + &String::from_utf8_lossy(failure.stdout()), + cred, + ) + .retry_reason(), + sandbox_driver::Error::Provider(provider) => { + match classify_message(&provider.message, cred) { + GitMessageClass::Retry(reason) => Some(reason), + GitMessageClass::Permanent => None, + GitMessageClass::Unknown => { + provider.retryable.then_some(GitRetryReason::TransientInfra) + } + } + } + sandbox_driver::Error::RateLimited { .. } | sandbox_driver::Error::Overloaded { .. } => { + Some(GitRetryReason::TransientInfra) + } + _ => None, + } +} + /// Backoff between attempts: 3s, then 9s. /// /// GitHub's guidance for token replication is to wait a few seconds and retry diff --git a/lib/components/fabro-sandbox/src/lib.rs b/lib/components/fabro-sandbox/src/lib.rs index 1424d8411..c6824d07f 100644 --- a/lib/components/fabro-sandbox/src/lib.rs +++ b/lib/components/fabro-sandbox/src/lib.rs @@ -1,17 +1,14 @@ pub mod config; pub mod error; -#[cfg(any(feature = "docker", feature = "daytona"))] pub mod from_environment; pub mod provider; pub mod sandbox; pub mod sandbox_spec; -#[cfg(any(feature = "docker", feature = "daytona"))] mod clone_source; mod git_retry; -#[cfg(any(feature = "docker", feature = "daytona", test))] mod managed_labels; mod push_credentials; @@ -29,7 +26,7 @@ pub mod reconnect; pub mod terminal; -#[cfg(feature = "docker")] +mod clone; pub mod docker; #[cfg(feature = "daytona")] @@ -39,8 +36,7 @@ pub mod daytona; pub mod test_support; pub use details::sandbox_details; -#[cfg(feature = "docker")] -pub use docker::{DockerSandbox, DockerSandboxOptions}; +pub use docker::{DockerSandboxOptions, attach_docker, check_docker_daemon, docker_sandbox}; pub use driver_sandbox::{DriverSandbox, local_sandbox}; pub use error::{Error, Result, default_redacted_output_tail, display_for_log}; pub use exec::{ExplicitEnvPolicy, SandboxExec, is_sensitive_env_var}; @@ -53,11 +49,9 @@ pub use git_retry::{ }; #[cfg(feature = "daytona")] pub use provider::daytona::DaytonaSandboxProvider; -#[cfg(feature = "docker")] -pub use provider::docker::DockerSandboxProvider; +pub use provider::driver::DriverInventoryProvider; pub use provider::{ - LocalSandboxProvider, SandboxCreateSpec, SandboxLookupError, SandboxProvider, - SandboxProviderRegistry, + LocalSandboxProvider, SandboxLookupError, SandboxProvider, SandboxProviderRegistry, }; pub use push_credentials::RefreshErrorKind; pub use reconnect::{reconnect, reconnect_for_run, reconnect_for_run_with_callback}; @@ -70,4 +64,4 @@ pub use sandbox::{ redacted_output_tail, setup_git_via_exec, shell_quote, }; pub use sandbox_spec::SandboxSpec; -pub use terminal::{TerminalSession, TerminalSize, open_terminal_for_run}; +pub use terminal::{DriverTerminalSession, TerminalSession, TerminalSize, open_terminal_for_run}; diff --git a/lib/components/fabro-sandbox/src/managed_labels.rs b/lib/components/fabro-sandbox/src/managed_labels.rs index 09b2dd96f..250b08c1c 100644 --- a/lib/components/fabro-sandbox/src/managed_labels.rs +++ b/lib/components/fabro-sandbox/src/managed_labels.rs @@ -7,12 +7,10 @@ pub(crate) const MANAGED_LABEL_VALUE: &str = "true"; pub(crate) const RUN_ID_LABEL: &str = "sh.fabro.run_id"; /// True when the provided label map carries the Fabro managed sentinel. -#[cfg(any(feature = "docker", feature = "daytona", test))] pub(crate) fn is_managed(labels: &HashMap) -> bool { labels.get(MANAGED_LABEL).map(String::as_str) == Some(MANAGED_LABEL_VALUE) } -#[cfg(any(feature = "docker", test))] pub(crate) fn for_run(run_id: Option<&RunId>) -> HashMap { let mut labels = HashMap::new(); insert_for_run(&mut labels, run_id); diff --git a/lib/components/fabro-sandbox/src/provider.rs b/lib/components/fabro-sandbox/src/provider.rs index 2201d4996..13e2c784b 100644 --- a/lib/components/fabro-sandbox/src/provider.rs +++ b/lib/components/fabro-sandbox/src/provider.rs @@ -1,15 +1,10 @@ #[cfg(feature = "daytona")] pub mod daytona; -#[cfg(feature = "docker")] -pub mod docker; +pub mod driver; use std::sync::Arc; use async_trait::async_trait; -#[cfg(any(feature = "docker", feature = "daytona"))] -use fabro_github::GitHubCredentials; -#[cfg(any(feature = "docker", feature = "daytona"))] -use fabro_types::RunId; use fabro_types::{ SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxProviderKind, SandboxProviderLookupError, @@ -17,39 +12,12 @@ use fabro_types::{ use fabro_util::error::collect_chain; use futures::future::join_all; -#[cfg(feature = "daytona")] -use crate::daytona::DaytonaConfig; -#[cfg(feature = "docker")] -use crate::docker::DockerSandboxOptions; - -pub enum SandboxCreateSpec { - Local, - #[cfg(feature = "docker")] - Docker { - config: DockerSandboxOptions, - github_app: Option, - run_id: Option, - clone_origin_url: Option, - clone_branch: Option, - }, - #[cfg(feature = "daytona")] - Daytona { - config: Box, - github_app: Option, - run_id: Option, - clone_origin_url: Option, - clone_branch: Option, - api_key: Option, - }, -} - #[async_trait] pub trait SandboxProvider: Send + Sync { fn kind(&self) -> SandboxProviderKind; async fn list(&self) -> crate::Result>; async fn get(&self, id: &str) -> crate::Result>; - async fn create(&self, spec: SandboxCreateSpec) -> crate::Result; async fn delete(&self, id: &str) -> crate::Result<()>; } @@ -168,12 +136,6 @@ impl SandboxProvider for LocalSandboxProvider { Ok(None) } - async fn create(&self, _spec: SandboxCreateSpec) -> crate::Result { - Err(crate::Error::message( - "local sandbox provider has no provider-managed inventory", - )) - } - async fn delete(&self, _id: &str) -> crate::Result<()> { Ok(()) } diff --git a/lib/components/fabro-sandbox/src/provider/daytona.rs b/lib/components/fabro-sandbox/src/provider/daytona.rs index 3d05a0429..14280e89e 100644 --- a/lib/components/fabro-sandbox/src/provider/daytona.rs +++ b/lib/components/fabro-sandbox/src/provider/daytona.rs @@ -4,10 +4,9 @@ use async_trait::async_trait; use fabro_static::EnvVars; use fabro_types::{SandboxInfo, SandboxProviderKind}; -use super::{SandboxCreateSpec, SandboxProvider}; -use crate::daytona::{self, DaytonaSandbox}; +use super::SandboxProvider; use crate::managed_labels::{self, MANAGED_LABEL, MANAGED_LABEL_VALUE}; -use crate::{Sandbox, details}; +use crate::{daytona, details}; const DAYTONA_LIST_PAGE_SIZE: i32 = 100; @@ -105,42 +104,6 @@ impl SandboxProvider for DaytonaSandboxProvider { ))) } - async fn create(&self, spec: SandboxCreateSpec) -> crate::Result { - let SandboxCreateSpec::Daytona { - config, - github_app, - run_id, - clone_origin_url, - clone_branch, - api_key, - } = spec - else { - return Err(crate::Error::message( - "Daytona sandbox provider can only create Daytona sandboxes", - )); - }; - - let api_key = api_key.or_else(|| self.api_key.clone()).ok_or_else(|| { - crate::Error::message(format!("{} is not configured", EnvVars::DAYTONA_API_KEY)) - })?; - let sandbox = DaytonaSandbox::new( - config.as_ref().clone(), - github_app, - run_id, - clone_origin_url, - clone_branch, - None, - None, - Some(api_key), - ) - .await?; - sandbox.initialize().await?; - let sdk_sandbox = sandbox.sandbox_handle().ok_or_else(|| { - crate::Error::message("Daytona sandbox was created but no SDK handle is available") - })?; - Ok(details::daytona::daytona_info_from_sdk_sandbox(sdk_sandbox)) - } - async fn delete(&self, id: &str) -> crate::Result<()> { let client = self.client().await?; let sandbox = match client.get(id).await { diff --git a/lib/components/fabro-sandbox/src/provider/docker.rs b/lib/components/fabro-sandbox/src/provider/docker.rs deleted file mode 100644 index 72a210f0e..000000000 --- a/lib/components/fabro-sandbox/src/provider/docker.rs +++ /dev/null @@ -1,179 +0,0 @@ -use std::collections::HashMap; - -use async_trait::async_trait; -use bollard::Docker; -use bollard::container::{InspectContainerOptions, ListContainersOptions, RemoveContainerOptions}; -use bollard::errors::Error as DockerError; -use bollard::models::ContainerInspectResponse; -use fabro_types::{SandboxInfo, SandboxProviderKind}; -use futures::future::try_join_all; - -use super::{SandboxCreateSpec, SandboxProvider}; -use crate::docker::DockerSandbox; -use crate::managed_labels::{self, MANAGED_LABEL, MANAGED_LABEL_VALUE}; -use crate::{Sandbox, details}; - -#[derive(Debug, Clone, Default)] -pub struct DockerSandboxProvider; - -impl DockerSandboxProvider { - pub fn new() -> Self { - Self - } - - pub async fn check_daemon() -> crate::Result<()> { - let docker = Self::docker_client()?; - docker - .ping() - .await - .map_err(|err| crate::Error::context("Failed to reach Docker daemon", err))?; - Ok(()) - } - - fn docker_client() -> crate::Result { - Docker::connect_with_local_defaults().map_err(crate::Error::docker_connect) - } -} - -#[async_trait] -impl SandboxProvider for DockerSandboxProvider { - fn kind(&self) -> SandboxProviderKind { - SandboxProviderKind::DOCKER - } - - async fn list(&self) -> crate::Result> { - let docker = Self::docker_client()?; - let mut filters = HashMap::new(); - filters.insert("label".to_string(), vec![format!( - "{MANAGED_LABEL}={MANAGED_LABEL_VALUE}" - )]); - let options = ListContainersOptions:: { - all: true, - filters, - ..Default::default() - }; - let containers = docker - .list_containers(Some(options)) - .await - .map_err(|err| crate::Error::context("Failed to list Docker containers", err))?; - - let ids: Vec = containers.into_iter().filter_map(|c| c.id).collect(); - // Daemon-side label filter already restricts to managed containers, so we - // can skip the per-inspect managed re-check. Run inspects concurrently on - // the shared Docker client to avoid a serial N+1 round-trip. - let inspects = try_join_all( - ids.iter() - .map(|id| docker.inspect_container(id, None::)), - ) - .await - .map_err(|err| crate::Error::context("Failed to inspect Docker container", err))?; - Ok(inspects - .iter() - .map(details::docker::docker_info_from_inspect) - .collect()) - } - - async fn get(&self, id: &str) -> crate::Result> { - let docker = Self::docker_client()?; - let Some(inspect) = inspect_container(&docker, id).await? else { - return Ok(None); - }; - if !managed_from_inspect(&inspect) { - return Ok(None); - } - Ok(Some(details::docker::docker_info_from_inspect(&inspect))) - } - - async fn create(&self, spec: SandboxCreateSpec) -> crate::Result { - let SandboxCreateSpec::Docker { - config, - github_app, - run_id, - clone_origin_url, - clone_branch, - } = spec - else { - return Err(crate::Error::message( - "Docker sandbox provider can only create Docker sandboxes", - )); - }; - - let sandbox = DockerSandbox::new( - config, - github_app.as_ref(), - run_id, - clone_origin_url, - clone_branch, - None, - None, - )?; - sandbox.initialize().await?; - let container_id = sandbox.container_identifier()?.to_string(); - self.get(&container_id).await?.ok_or_else(|| { - crate::Error::message(format!( - "Docker sandbox '{container_id}' was created but is not visible in provider inventory" - )) - }) - } - - async fn delete(&self, id: &str) -> crate::Result<()> { - let docker = Self::docker_client()?; - let Some(inspect) = inspect_container(&docker, id).await? else { - return Ok(()); - }; - if !managed_from_inspect(&inspect) { - return Err(crate::Error::message(format!( - "Refusing to delete Docker container '{id}' because it is missing label {MANAGED_LABEL}={MANAGED_LABEL_VALUE}" - ))); - } - - let container_id = inspect.id.as_deref().unwrap_or(id); - docker - .remove_container( - container_id, - Some(RemoveContainerOptions { - force: true, - ..Default::default() - }), - ) - .await - .map_err(|err| { - crate::Error::context( - format!("Failed to remove Docker container '{container_id}'"), - err, - ) - }) - } -} - -async fn inspect_container( - docker: &Docker, - id: &str, -) -> crate::Result> { - match docker - .inspect_container(id, None::) - .await - { - Ok(inspect) => Ok(Some(inspect)), - Err(err) if docker_not_found(&err) => Ok(None), - Err(err) => Err(crate::Error::context( - format!("Failed to inspect Docker container '{id}'"), - err, - )), - } -} - -fn managed_from_inspect(inspect: &ContainerInspectResponse) -> bool { - inspect - .config - .as_ref() - .and_then(|config| config.labels.as_ref()) - .is_some_and(managed_labels::is_managed) -} - -fn docker_not_found(error: &DockerError) -> bool { - matches!(error, DockerError::DockerResponseServerError { - status_code: 404, - .. - }) -} diff --git a/lib/components/fabro-sandbox/src/provider/driver.rs b/lib/components/fabro-sandbox/src/provider/driver.rs new file mode 100644 index 000000000..e139911ed --- /dev/null +++ b/lib/components/fabro-sandbox/src/provider/driver.rs @@ -0,0 +1,246 @@ +//! Fabro-managed inventory over a sandbox-driver provider. +//! +//! Lists and looks up the sandboxes fabro created, identified by fabro's +//! own `sh.fabro.managed` label. The driver marks every sandbox it creates +//! with its own label too, but that covers every application on the same +//! daemon or account; fabro filters on its label and refuses to delete a +//! sandbox that does not carry it. + +use std::collections::BTreeMap; +use std::sync::Arc; + +use async_trait::async_trait; +use fabro_types::settings::server::ServerSandboxProviderSettings; +use fabro_types::{SandboxInfo, SandboxProviderKind}; +use sandbox_driver::{SandboxFilter, SandboxId, SandboxProvider as DriverProvider}; +use tokio::sync::OnceCell; + +use super::SandboxProvider; +use crate::details; +use crate::driver::{ConnectedProvider, ProviderConnectOptions, connect_provider}; +use crate::managed_labels::{MANAGED_LABEL, MANAGED_LABEL_VALUE}; + +/// How the driver provider behind the inventory is obtained. +enum Connection { + Connected(Arc), + /// Connected on first use, so a registry can be assembled synchronously + /// and a provider that is down surfaces as a lookup error rather than a + /// startup failure. + Lazy(Box), +} + +struct LazyConnection { + settings: ServerSandboxProviderSettings, + options: ProviderConnectOptions, + provider: OnceCell>, +} + +pub struct DriverInventoryProvider { + kind: SandboxProviderKind, + connection: Connection, +} + +impl DriverInventoryProvider { + #[must_use] + pub fn new(connected: ConnectedProvider) -> Self { + Self { + kind: connected.kind, + connection: Connection::Connected(connected.provider), + } + } + + /// An inventory over a provider connected through + /// [`connect_provider`] on first use. + #[must_use] + pub fn lazy( + kind: SandboxProviderKind, + settings: ServerSandboxProviderSettings, + options: ProviderConnectOptions, + ) -> Self { + Self { + kind, + connection: Connection::Lazy(Box::new(LazyConnection { + settings, + options, + provider: OnceCell::new(), + })), + } + } + + async fn provider(&self) -> crate::Result<&Arc> { + match &self.connection { + Connection::Connected(provider) => Ok(provider), + Connection::Lazy(lazy) => { + lazy.provider + .get_or_try_init(|| async { + connect_provider(&self.kind, &lazy.settings, &lazy.options) + .await + .map(|connected| connected.provider) + .map_err(|error| { + crate::Error::context( + format!("Failed to connect to the {} provider", self.kind), + error, + ) + }) + }) + .await + } + } + } + + fn managed_filter() -> SandboxFilter { + let mut filter = SandboxFilter::default(); + filter + .labels + .insert(MANAGED_LABEL.to_string(), MANAGED_LABEL_VALUE.to_string()); + filter + } + + fn is_managed(labels: &BTreeMap) -> bool { + labels.get(MANAGED_LABEL).map(String::as_str) == Some(MANAGED_LABEL_VALUE) + } + + async fn describe_managed( + &self, + id: &str, + ) -> crate::Result> { + // 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 self.provider().await?.attach(&sandbox_id, None).await { + Ok(handle) => handle, + Err(sandbox_driver::Error::NotFound { .. }) => return Ok(None), + Err(error) => { + return Err(crate::Error::context( + format!("Failed to look up {} sandbox '{id}'", self.kind), + error, + )); + } + }; + let status = handle.describe().await.map_err(|error| { + crate::Error::context( + format!("Failed to describe {} sandbox '{id}'", self.kind), + error, + ) + })?; + if status.state == sandbox_driver::SandboxState::Deleted + || !Self::is_managed(&status.labels) + { + return Ok(None); + } + Ok(Some(status)) + } +} + +#[async_trait] +impl SandboxProvider for DriverInventoryProvider { + fn kind(&self) -> SandboxProviderKind { + self.kind.clone() + } + + async fn list(&self) -> crate::Result> { + let statuses = self + .provider() + .await? + .list(&Self::managed_filter()) + .await + .map_err(|error| { + crate::Error::context(format!("Failed to list {} sandboxes", self.kind), error) + })?; + Ok(statuses + .iter() + // The filter is a request; a provider that cannot filter on + // labels returns everything, so the label is checked again. + .filter(|status| Self::is_managed(&status.labels)) + .map(|status| details::info_from_status(&self.kind, status)) + .collect()) + } + + async fn get(&self, id: &str) -> crate::Result> { + Ok(self + .describe_managed(id) + .await? + .map(|status| details::info_from_status(&self.kind, &status))) + } + + async fn delete(&self, id: &str) -> crate::Result<()> { + let Some(status) = self.describe_managed(id).await? else { + // Missing, already deleted, or not fabro's: the first two are + // idempotent successes and the third must never be deleted here, + // so distinguish them for the caller. + if let Ok(sandbox_id) = SandboxId::try_new(id) { + if let Ok(handle) = self.provider().await?.attach(&sandbox_id, None).await { + let status = handle.describe().await?; + if status.state != sandbox_driver::SandboxState::Deleted { + return Err(crate::Error::message(format!( + "Refusing to delete {} sandbox '{id}' because it is missing label {MANAGED_LABEL}={MANAGED_LABEL_VALUE}", + self.kind + ))); + } + } + } + return Ok(()); + }; + self.provider() + .await? + .delete(&status.id, None) + .await + .map_err(|error| { + crate::Error::context( + format!("Failed to delete {} sandbox '{id}'", self.kind), + error, + ) + }) + } +} + +#[cfg(test)] +mod tests { + use sandbox_driver::{SandboxSource, SandboxSpec}; + use sandbox_driver_host::HostProvider; + + use super::*; + + fn inventory() -> (DriverInventoryProvider, Arc) { + let host = Arc::new(HostProvider::new()); + let provider = DriverInventoryProvider::new(ConnectedProvider { + kind: SandboxProviderKind::try_new("host").unwrap(), + provider: host.clone(), + }); + (provider, host) + } + + #[tokio::test] + async fn lists_and_deletes_only_fabro_managed_sandboxes() { + let (inventory, host) = inventory(); + let ours = host + .create( + &SandboxSpec::new(SandboxSource::HostDirectory).label(MANAGED_LABEL, "true"), + None, + ) + .await + .unwrap(); + let theirs = host + .create(&SandboxSpec::new(SandboxSource::HostDirectory), None) + .await + .unwrap(); + + let listed = inventory.list().await.unwrap(); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].id, ours.id().as_str()); + assert_eq!(listed[0].provider.as_str(), "host"); + assert!(inventory.get(ours.id().as_str()).await.unwrap().is_some()); + assert!(inventory.get(theirs.id().as_str()).await.unwrap().is_none()); + + let refused = inventory.delete(theirs.id().as_str()).await.unwrap_err(); + assert!( + refused.to_string().contains("Refusing to delete"), + "{refused}" + ); + inventory.delete(ours.id().as_str()).await.unwrap(); + assert!(inventory.get(ours.id().as_str()).await.unwrap().is_none()); + inventory.delete(ours.id().as_str()).await.unwrap(); + inventory.delete("never-existed").await.unwrap(); + } +} diff --git a/lib/components/fabro-sandbox/src/reconnect.rs b/lib/components/fabro-sandbox/src/reconnect.rs index 89fd96cf2..34941addf 100644 --- a/lib/components/fabro-sandbox/src/reconnect.rs +++ b/lib/components/fabro-sandbox/src/reconnect.rs @@ -7,12 +7,10 @@ use std::path::PathBuf; use anyhow::{Context, Result, bail}; use fabro_types::{BundledProvider, RunId, RunSandboxInstance}; -use crate::SandboxEventCallback; #[cfg(feature = "daytona")] use crate::daytona::DaytonaSandbox; -#[cfg(feature = "docker")] -use crate::docker::DockerSandbox; use crate::driver_sandbox::local_sandbox; +use crate::{SandboxEventCallback, docker}; /// Reconnect to a sandbox from a saved record. /// @@ -66,17 +64,15 @@ pub async fn reconnect_for_run_with_callback( } Ok(Box::new(sandbox)) } - #[cfg(feature = "docker")] Some(BundledProvider::Docker) => { let repo_cloned = runtime .repo_cloned .context("Docker run sandbox missing repo_cloned metadata")?; - let mut sandbox = DockerSandbox::reconnect( + let mut sandbox = docker::attach_docker( &runtime.id, repo_cloned, runtime.working_directory.clone(), runtime.clone_origin_url.clone(), - runtime.clone_branch.clone(), run_id, ) .await @@ -86,8 +82,6 @@ pub async fn reconnect_for_run_with_callback( } Ok(Box::new(sandbox)) } - #[cfg(not(feature = "docker"))] - Some(BundledProvider::Docker) => bail!("Docker sandbox support is not enabled"), #[cfg(feature = "daytona")] Some(BundledProvider::Daytona) => { let repo_cloned = runtime diff --git a/lib/components/fabro-sandbox/src/sandbox.rs b/lib/components/fabro-sandbox/src/sandbox.rs index 3d99ec896..352d372bf 100644 --- a/lib/components/fabro-sandbox/src/sandbox.rs +++ b/lib/components/fabro-sandbox/src/sandbox.rs @@ -13,7 +13,7 @@ use fabro_types::{CommandOutputStream, CommandTermination}; use fabro_util::shell; use fabro_util::workspace_glob::WorkspaceGlob; use serde::{Deserialize, Serialize}; -use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite}; use tokio::sync::Mutex as TokioMutex; use tokio::task::JoinHandle; use tokio::time; @@ -31,11 +31,9 @@ pub const DEFAULT_EXEC_OUTPUT_TAIL_BYTES: usize = 8 * 1024; pub(crate) const BASH_PROBE_TIMEOUT_MS: u64 = 10_000; /// Bash path required by Linux-backed remote sandbox providers. -#[cfg(any(feature = "docker", feature = "daytona"))] pub(crate) const REMOTE_BASH: &str = "/bin/bash"; /// Timeout for provider-neutral remote file traversal. -#[cfg(any(feature = "docker", feature = "daytona"))] pub(crate) const REMOTE_WALK_TIMEOUT_MS: u64 = 30_000; /// Environment variable Bash consults for non-interactive startup source. @@ -946,42 +944,6 @@ impl<'a> ExecStreamingRequest<'a> { } } -pub(crate) async fn write_process_stdin(mut writer: W, stdin: &[u8]) -> crate::Result<()> -where - W: AsyncWrite + Unpin, -{ - // A command that stops reading its input (`head -1`, an early exit) is - // not an error; its exit code is the authoritative result. Local pipes - // surface that as `BrokenPipe`, remote transports (a TCP Docker daemon) - // as `ConnectionReset`/`ConnectionAborted`. - fn command_stopped_reading(err: &std::io::Error) -> bool { - matches!( - err.kind(), - std::io::ErrorKind::BrokenPipe - | std::io::ErrorKind::ConnectionReset - | std::io::ErrorKind::ConnectionAborted - ) - } - - if let Err(err) = writer.write_all(stdin).await { - if !command_stopped_reading(&err) { - return Err(crate::Error::context( - "Failed to write command standard input", - err, - )); - } - } - if let Err(err) = writer.shutdown().await { - if !command_stopped_reading(&err) { - return Err(crate::Error::context( - "Failed to close command standard input", - err, - )); - } - } - Ok(()) -} - pub(crate) async fn replay_exec_result( mut result: ExecResult, streams_separated: bool, @@ -1603,7 +1565,6 @@ pub(crate) fn join_sandbox_path(base: &str, relative_path: &str) -> String { format!("{}/{relative_path}", base.trim_end_matches('/')) } -#[cfg(any(feature = "docker", feature = "daytona"))] pub(crate) fn build_remote_walk_command( base: &str, relative_start: &str, @@ -1637,7 +1598,6 @@ pub(crate) fn build_remote_walk_command( command } -#[cfg(any(feature = "docker", feature = "daytona"))] pub(crate) fn parse_remote_walk_output( base: &str, relative_start: &str, diff --git a/lib/components/fabro-sandbox/src/sandbox_spec.rs b/lib/components/fabro-sandbox/src/sandbox_spec.rs index 162dc4f01..4c42de820 100644 --- a/lib/components/fabro-sandbox/src/sandbox_spec.rs +++ b/lib/components/fabro-sandbox/src/sandbox_spec.rs @@ -2,7 +2,6 @@ use std::path::PathBuf; use std::sync::Arc; use anyhow::Context as _; -#[cfg(any(feature = "docker", feature = "daytona"))] use fabro_github::GitHubCredentials; #[allow( unused_imports, @@ -10,21 +9,17 @@ use fabro_github::GitHubCredentials; )] use fabro_types::{RunId, RunSandboxInstance, RunSandboxRuntime, SandboxProviderKind}; -#[cfg(any(feature = "docker", feature = "daytona"))] -use crate::clone_source; #[cfg(feature = "daytona")] use crate::daytona::{self, DaytonaConfig, DaytonaSandbox}; -#[cfg(feature = "docker")] -use crate::docker::{self, DockerSandbox, DockerSandboxOptions}; +use crate::docker::{self, DockerSandboxOptions}; use crate::driver_sandbox::local_sandbox; -use crate::{Sandbox, SandboxEventCallback}; +use crate::{Sandbox, SandboxEventCallback, clone_source}; /// Options for sandbox initialization and construction. pub enum SandboxSpec { Local { working_directory: PathBuf, }, - #[cfg(feature = "docker")] Docker { config: DockerSandboxOptions, github_app: Option, @@ -51,7 +46,6 @@ impl SandboxSpec { pub fn provider(&self) -> SandboxProviderKind { match self { Self::Local { .. } => SandboxProviderKind::LOCAL, - #[cfg(feature = "docker")] Self::Docker { .. } => SandboxProviderKind::DOCKER, #[cfg(feature = "daytona")] Self::Daytona { .. } => SandboxProviderKind::DAYTONA, @@ -61,7 +55,6 @@ impl SandboxSpec { pub fn provider_name(&self) -> &'static str { match self { Self::Local { .. } => "local", - #[cfg(feature = "docker")] Self::Docker { .. } => "docker", #[cfg(feature = "daytona")] Self::Daytona { .. } => "daytona", @@ -85,7 +78,6 @@ impl SandboxSpec { }; match self { - #[cfg(feature = "docker")] Self::Docker { config, clone_origin_url, @@ -198,7 +190,6 @@ impl SandboxSpec { } Ok(Arc::new(sandbox)) } - #[cfg(feature = "docker")] Self::Docker { config, github_app, @@ -208,7 +199,7 @@ impl SandboxSpec { clone_tag, clone_commit_sha, } => { - let mut sandbox = DockerSandbox::new( + let mut sandbox = docker::docker_sandbox( config.clone(), github_app.as_ref(), *run_id, @@ -217,6 +208,7 @@ impl SandboxSpec { clone_tag.clone(), clone_commit_sha.clone(), ) + .await .context("Failed to create Docker sandbox")?; if let Some(callback) = event_callback { sandbox.set_event_callback(callback); @@ -255,7 +247,6 @@ impl SandboxSpec { } } -#[cfg(any(feature = "docker", feature = "daytona"))] fn runtime_layout_metadata( repo_cloned: Option, clone_origin_url: Option<&str>, @@ -270,15 +261,11 @@ fn runtime_layout_metadata( #[cfg(test)] mod tests { - #[cfg(feature = "docker")] use fabro_types::RunId; - #[cfg(feature = "docker")] use super::*; - #[cfg(feature = "docker")] use crate::test_support::MockSandbox; - #[cfg(feature = "docker")] #[test] fn docker_run_sandbox_persists_layout_metadata_for_cloned_repo() { let spec = SandboxSpec::Docker { @@ -317,7 +304,6 @@ mod tests { assert!(runtime_json.get("clone_commit_sha").is_none()); } - #[cfg(feature = "docker")] #[tokio::test] async fn invalid_exact_checkout_spec_fails_before_provider_connection() { let spec = SandboxSpec::Docker { @@ -344,7 +330,6 @@ mod tests { assert!(!format!("{error:#}").contains("Docker daemon")); } - #[cfg(feature = "docker")] #[test] fn docker_run_sandbox_omits_primary_repo_metadata_for_empty_workspace() { let spec = SandboxSpec::Docker { diff --git a/lib/components/fabro-sandbox/src/terminal.rs b/lib/components/fabro-sandbox/src/terminal.rs index e5eb6b94b..de7633728 100644 --- a/lib/components/fabro-sandbox/src/terminal.rs +++ b/lib/components/fabro-sandbox/src/terminal.rs @@ -3,12 +3,9 @@ use async_trait::async_trait; use fabro_static::EnvVars; use fabro_types::{BundledProvider, RunId, RunSandboxInstance}; -#[cfg(any(feature = "daytona", feature = "docker"))] -use crate::Sandbox; #[cfg(feature = "daytona")] use crate::daytona::{DEFAULT_DAYTONA_API_URL, DaytonaSandbox}; -#[cfg(feature = "docker")] -use crate::docker::DockerSandbox; +use crate::{Sandbox, docker}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct TerminalSize { @@ -33,6 +30,52 @@ pub trait TerminalSession: Send + Sync { async fn close(&self) -> crate::Result<()>; } +/// A terminal over the driver's Pty facet. +pub struct DriverTerminalSession { + session: Box, +} + +impl DriverTerminalSession { + #[must_use] + pub fn new(session: Box) -> Self { + Self { session } + } +} + +#[async_trait] +impl TerminalSession for DriverTerminalSession { + async fn write_input(&self, bytes: &[u8]) -> crate::Result<()> { + self.session + .write_input(bytes) + .await + .map_err(|err| crate::Error::context("Failed to write terminal input", err)) + } + + async fn read_output(&self) -> crate::Result>> { + self.session + .read_output() + .await + .map_err(|err| crate::Error::context("Failed to read terminal output", err)) + } + + async fn resize(&self, size: TerminalSize) -> crate::Result<()> { + self.session + .resize(sandbox_driver::PtySize { + rows: size.rows, + cols: size.cols, + }) + .await + .map_err(|err| crate::Error::context("Failed to resize terminal", err)) + } + + async fn close(&self) -> crate::Result<()> { + self.session + .close() + .await + .map_err(|err| crate::Error::context("Failed to close terminal", err)) + } +} + pub async fn open_terminal_for_run( record: &RunSandboxInstance, daytona_api_key: Option, @@ -40,14 +83,9 @@ pub async fn open_terminal_for_run( run_id: Option, size: TerminalSize, ) -> crate::Result> { - #[cfg(any(feature = "daytona", feature = "docker"))] let runtime = &record.runtime; #[cfg(not(feature = "daytona"))] let _ = (&daytona_api_key, &daytona_organization_id); - #[cfg(not(feature = "docker"))] - let _ = &run_id; - #[cfg(not(any(feature = "daytona", feature = "docker")))] - let _ = size; match record.provider.bundled() { #[cfg(feature = "daytona")] @@ -81,28 +119,21 @@ pub async fn open_terminal_for_run( Some(BundledProvider::Daytona) => Err(crate::Error::message( "Daytona sandbox support is not enabled", )), - #[cfg(feature = "docker")] Some(BundledProvider::Docker) => { let repo_cloned = runtime.repo_cloned.ok_or_else(|| { crate::Error::message("Docker run sandbox is missing clone metadata") })?; - let sandbox = DockerSandbox::reconnect( + let sandbox = docker::attach_docker( &runtime.id, repo_cloned, runtime.working_directory.clone(), runtime.clone_origin_url.clone(), - runtime.clone_branch.clone(), run_id, ) .await?; sandbox.activate().await?; - let session = DockerTerminalSession::open(&sandbox, size).await?; - Ok(Box::new(session)) + Ok(Box::new(sandbox.open_terminal(size).await?)) } - #[cfg(not(feature = "docker"))] - Some(BundledProvider::Docker) => Err(crate::Error::message( - "Docker sandbox support is not enabled", - )), Some(BundledProvider::Local) => Err(crate::Error::message( "Local sandboxes do not support embedded terminals", )), @@ -680,213 +711,3 @@ mod daytona_terminal { #[cfg(feature = "daytona")] use daytona_terminal::DaytonaTerminalSession; - -#[cfg(feature = "docker")] -mod docker_terminal { - use std::pin::Pin; - use std::sync::atomic::{AtomicU64, Ordering}; - - use async_trait::async_trait; - use bollard::Docker; - use bollard::container::LogOutput; - use bollard::errors::Error as DockerError; - use bollard::exec::{CreateExecOptions, ResizeExecOptions, StartExecResults}; - use futures::{Stream, StreamExt}; - use tokio::io::{AsyncWrite, AsyncWriteExt}; - use tokio::sync::Mutex; - - use super::{TerminalSession, TerminalSize}; - use crate::Sandbox; - use crate::docker::DockerSandbox; - - type DockerInput = Pin>; - type DockerOutput = Pin> + Send>>; - - pub(super) struct DockerTerminalSession { - docker: Docker, - container_id: String, - exec_id: String, - pid_file: String, - input: Mutex>, - output: Mutex>, - closed: Mutex, - } - - impl DockerTerminalSession { - pub(super) async fn open( - sandbox: &DockerSandbox, - size: TerminalSize, - ) -> crate::Result { - let docker = sandbox.docker_client(); - let container_id = sandbox.container_identifier()?.to_string(); - let pid_file = format!("/tmp/fabro-terminal-{}.pid", uuid_fragment()); - let exec_opts = docker_terminal_exec_options(sandbox.working_directory(), &pid_file); - let exec = docker - .create_exec(&container_id, exec_opts) - .await - .map_err(|err| { - crate::Error::context("Failed to create Docker terminal exec", err) - })?; - let exec_id = exec.id; - let start = docker.start_exec(&exec_id, None).await.map_err(|err| { - crate::Error::context("Failed to start Docker terminal exec", err) - })?; - let StartExecResults::Attached { output, input } = start else { - return Err(crate::Error::message("Docker terminal exec did not attach")); - }; - docker - .resize_exec(&exec_id, ResizeExecOptions { - height: size.rows, - width: size.cols, - }) - .await - .map_err(|err| { - crate::Error::context("Failed to resize Docker terminal exec", err) - })?; - Ok(Self { - docker, - container_id, - exec_id, - pid_file, - input: Mutex::new(Some(input)), - output: Mutex::new(Some(output)), - closed: Mutex::new(false), - }) - } - - async fn kill_shell(&self) -> crate::Result<()> { - let command = format!( - "if [ -f {pid_file} ]; then kill -TERM \"$(cat {pid_file})\" 2>/dev/null || true; rm -f {pid_file}; fi", - pid_file = crate::shell_quote(&self.pid_file), - ); - let exec = self - .docker - .create_exec(&self.container_id, CreateExecOptions { - cmd: Some(vec!["sh".to_string(), "-lc".to_string(), command]), - attach_stdout: Some(false), - attach_stderr: Some(false), - ..Default::default() - }) - .await - .map_err(|err| { - crate::Error::context("Failed to create Docker terminal cleanup exec", err) - })?; - self.docker - .start_exec(&exec.id, None) - .await - .map_err(|err| { - crate::Error::context("Failed to run Docker terminal cleanup exec", err) - })?; - Ok(()) - } - } - - #[async_trait] - impl TerminalSession for DockerTerminalSession { - async fn write_input(&self, bytes: &[u8]) -> crate::Result<()> { - let mut input = self.input.lock().await; - let Some(input) = input.as_mut() else { - return Ok(()); - }; - input - .write_all(bytes) - .await - .map_err(|err| crate::Error::context("Failed to write Docker terminal input", err)) - } - - async fn read_output(&self) -> crate::Result>> { - let mut output = self.output.lock().await; - let Some(output) = output.as_mut() else { - return Ok(None); - }; - match output.next().await { - Some(Ok(chunk)) => Ok(Some(chunk.into_bytes().to_vec())), - Some(Err(err)) => Err(crate::Error::context( - "Failed to read Docker terminal output", - err, - )), - None => Ok(None), - } - } - - async fn resize(&self, size: TerminalSize) -> crate::Result<()> { - self.docker - .resize_exec(&self.exec_id, ResizeExecOptions { - height: size.rows, - width: size.cols, - }) - .await - .map_err(|err| crate::Error::context("Failed to resize Docker terminal exec", err)) - } - - async fn close(&self) -> crate::Result<()> { - let mut closed = self.closed.lock().await; - if *closed { - return Ok(()); - } - *closed = true; - drop(closed); - let _ = self.input.lock().await.take(); - let _ = self.output.lock().await.take(); - self.kill_shell().await - } - } - - fn docker_terminal_exec_options( - working_directory: &str, - pid_file: &str, - ) -> CreateExecOptions { - let command = format!( - "printf '%s\\n' $$ > {pid_file}; exec sh -l", - pid_file = crate::shell_quote(pid_file), - ); - CreateExecOptions { - attach_stdin: Some(true), - attach_stdout: Some(true), - attach_stderr: Some(true), - tty: Some(true), - cmd: Some(vec!["sh".to_string(), "-lc".to_string(), command]), - working_dir: Some(working_directory.to_string()), - env: Some(vec![ - "TERM=xterm-256color".to_string(), - "LANG=C.UTF-8".to_string(), - ]), - ..Default::default() - } - } - - static DOCKER_TERMINAL_COUNTER: AtomicU64 = AtomicU64::new(1); - - fn uuid_fragment() -> String { - format!( - "{:016x}", - DOCKER_TERMINAL_COUNTER.fetch_add(1, Ordering::Relaxed) - ) - } - - #[cfg(test)] - mod tests { - use super::*; - - #[test] - fn docker_terminal_exec_options_attach_tty_and_workspace_env() { - let options = docker_terminal_exec_options("/workspace", "/tmp/fabro-terminal.pid"); - assert_eq!(options.attach_stdin, Some(true)); - assert_eq!(options.attach_stdout, Some(true)); - assert_eq!(options.attach_stderr, Some(true)); - assert_eq!(options.tty, Some(true)); - assert_eq!(options.working_dir.as_deref(), Some("/workspace")); - assert_eq!( - options.env, - Some(vec![ - "TERM=xterm-256color".to_string(), - "LANG=C.UTF-8".to_string() - ]) - ); - assert!(options.cmd.unwrap().join(" ").contains("exec sh -l")); - } - } -} - -#[cfg(feature = "docker")] -use docker_terminal::DockerTerminalSession; diff --git a/lib/components/fabro-sandbox/src/test_support.rs b/lib/components/fabro-sandbox/src/test_support.rs index 85ffe96c6..19e5aa6c1 100644 --- a/lib/components/fabro-sandbox/src/test_support.rs +++ b/lib/components/fabro-sandbox/src/test_support.rs @@ -741,7 +741,7 @@ mod fake_provider { SandboxTimestamps, }; - use crate::provider::{SandboxCreateSpec, SandboxProvider, SandboxProviderRegistry}; + use crate::provider::{SandboxProvider, SandboxProviderRegistry}; #[derive(Clone)] pub enum FakeList { @@ -789,10 +789,6 @@ mod fake_provider { } } - async fn create(&self, _spec: SandboxCreateSpec) -> crate::Result { - Err(crate::Error::message("not implemented")) - } - async fn delete(&self, _id: &str) -> crate::Result<()> { Ok(()) } diff --git a/lib/components/fabro-sandbox/tests/docker_streaming.rs b/lib/components/fabro-sandbox/tests/docker_streaming.rs index 0ee733f38..d8e074a46 100644 --- a/lib/components/fabro-sandbox/tests/docker_streaming.rs +++ b/lib/components/fabro-sandbox/tests/docker_streaming.rs @@ -1,13 +1,25 @@ -#![cfg(feature = "docker")] +//! Docker sandbox behaviour through the sandbox-driver Docker provider. use std::sync::Arc; -use bollard::Docker; use fabro_sandbox::{ - CommandOutputCallback, DockerSandbox, DockerSandboxOptions, ExecStreamingRequest, Sandbox, + CommandOutputCallback, DockerSandboxOptions, ExecStreamingRequest, Sandbox, docker_sandbox, }; +use tokio::process::Command; use tokio::sync::Mutex; +/// Whether a Docker daemon answers and has `image` locally. The tests are +/// skipped (not failed) otherwise, matching the ignore reason. +async fn docker_image_available(image: &str) -> bool { + Command::new("docker") + .args(["image", "inspect", image]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .await + .is_ok_and(|status| status.success()) +} + fn capture_bytes(chunks: Arc>>) -> CommandOutputCallback { Arc::new(move |_stream, bytes| { let chunks = Arc::clone(&chunks); @@ -22,14 +34,11 @@ fn capture_bytes(chunks: Arc>>) -> CommandOutputCallback { #[ignore = "requires real Docker container lifecycle; run explicitly when changing Docker exec integration"] async fn streaming_timeout_terminates_docker_exec_before_returning() { let image = "buildpack-deps:noble"; - let Ok(docker) = Docker::connect_with_local_defaults() else { - return; - }; - if docker.inspect_image(image).await.is_err() { + if !docker_image_available(image).await { return; } - let sandbox = DockerSandbox::new( + let sandbox = docker_sandbox( DockerSandboxOptions { image: image.to_string(), auto_pull: false, @@ -43,6 +52,7 @@ async fn streaming_timeout_terminates_docker_exec_before_returning() { None, None, ) + .await .expect("docker sandbox should construct"); sandbox .initialize() @@ -96,14 +106,11 @@ async fn streaming_timeout_terminates_docker_exec_before_returning() { #[ignore = "requires real Docker container lifecycle; run explicitly when changing Docker exec integration"] async fn streaming_command_receives_exact_stdin_and_eof() { let image = "buildpack-deps:noble"; - let Ok(docker) = Docker::connect_with_local_defaults() else { - return; - }; - if docker.inspect_image(image).await.is_err() { + if !docker_image_available(image).await { return; } - let sandbox = DockerSandbox::new( + let sandbox = docker_sandbox( DockerSandboxOptions { image: image.to_string(), auto_pull: false, @@ -117,6 +124,7 @@ async fn streaming_command_receives_exact_stdin_and_eof() { None, None, ) + .await .expect("docker sandbox should construct"); sandbox .initialize() @@ -159,14 +167,11 @@ async fn streaming_command_receives_exact_stdin_and_eof() { #[ignore = "requires real Docker container lifecycle, image, network, and a public GitHub clone"] async fn cloned_docker_sandbox_uses_repos_checkout_and_workspace_symlink() { let image = "buildpack-deps:noble"; - let Ok(docker) = Docker::connect_with_local_defaults() else { - return; - }; - if docker.inspect_image(image).await.is_err() { + if !docker_image_available(image).await { return; } - let sandbox = DockerSandbox::new( + let sandbox = docker_sandbox( DockerSandboxOptions { image: image.to_string(), auto_pull: false, @@ -180,6 +185,7 @@ async fn cloned_docker_sandbox_uses_repos_checkout_and_workspace_symlink() { None, None, ) + .await .expect("docker sandbox should construct"); sandbox .initialize() @@ -226,14 +232,11 @@ async fn cloned_docker_sandbox_uses_repos_checkout_and_workspace_symlink() { #[ignore = "requires real Docker container lifecycle; run explicitly when changing Docker exec integration"] async fn docker_runs_clean_bash_through_both_command_paths() { let image = "buildpack-deps:noble"; - let Ok(docker) = Docker::connect_with_local_defaults() else { - return; - }; - if docker.inspect_image(image).await.is_err() { + if !docker_image_available(image).await { return; } - let sandbox = DockerSandbox::new( + let sandbox = docker_sandbox( DockerSandboxOptions { image: image.to_string(), auto_pull: false, @@ -248,6 +251,7 @@ async fn docker_runs_clean_bash_through_both_command_paths() { None, None, ) + .await .expect("docker sandbox should construct"); sandbox .initialize() @@ -322,14 +326,11 @@ async fn docker_runs_clean_bash_through_both_command_paths() { #[ignore = "requires real Docker container lifecycle; run explicitly when changing Sandbox::glob"] async fn docker_glob_matches_patterns_containing_a_path_separator() { let image = "buildpack-deps:noble"; - let Ok(docker) = Docker::connect_with_local_defaults() else { - return; - }; - if docker.inspect_image(image).await.is_err() { + if !docker_image_available(image).await { return; } - let sandbox = DockerSandbox::new( + let sandbox = docker_sandbox( DockerSandboxOptions { image: image.to_string(), auto_pull: false, @@ -343,6 +344,7 @@ async fn docker_glob_matches_patterns_containing_a_path_separator() { None, None, ) + .await .expect("docker sandbox should construct"); sandbox .initialize() @@ -408,14 +410,11 @@ async fn docker_glob_matches_patterns_containing_a_path_separator() { #[ignore = "requires real Docker container lifecycle; run explicitly when changing Docker runtime directory setup"] async fn docker_runtime_directory_is_private_and_outside_workspace() { let image = "buildpack-deps:noble"; - let Ok(docker) = Docker::connect_with_local_defaults() else { - return; - }; - if docker.inspect_image(image).await.is_err() { + if !docker_image_available(image).await { return; } - let sandbox = DockerSandbox::new( + let sandbox = docker_sandbox( DockerSandboxOptions { image: image.to_string(), auto_pull: false, @@ -429,6 +428,7 @@ async fn docker_runtime_directory_is_private_and_outside_workspace() { None, None, ) + .await .expect("docker sandbox should construct"); sandbox .initialize() diff --git a/lib/components/fabro-sandbox/tests/driver_bench.rs b/lib/components/fabro-sandbox/tests/driver_bench.rs index 1536b1423..56952b342 100644 --- a/lib/components/fabro-sandbox/tests/driver_bench.rs +++ b/lib/components/fabro-sandbox/tests/driver_bench.rs @@ -4,9 +4,9 @@ //! Three comparisons, each over the same medium repository (fabro's own //! `lib/` tree, about 1,100 Rust files): //! -//! - Docker file reads and content search: fabro's `DockerSandbox` (archive API -//! reads, `docker exec` grep) against the driver `DockerProvider` (archive -//! API reads, exec-derived search) in-process. +//! - Docker file reads and content search: fabro's driver-backed Docker sandbox +//! (its path resolution and result shaping) against the bare driver +//! `DockerProvider` in-process. //! - Host tool calls: fabro's local sandbox against the driver `HostProvider` //! in-process, to confirm no regression on the path every local run takes. //! - The wire: the driver Host and Docker providers served over the JSON-RPC @@ -15,10 +15,9 @@ //! //! Ignored: it needs a Docker daemon with `buildpack-deps:noble` present and //! takes a minute. Run with -//! `cargo nextest run -p fabro-sandbox --features docker --test driver_bench -//! --run-ignored only --no-capture`. +//! `cargo nextest run -p fabro-sandbox --test driver_bench --run-ignored only +//! --no-capture`. -#![cfg(feature = "docker")] #![allow( clippy::print_stderr, clippy::cast_precision_loss, @@ -36,8 +35,7 @@ use std::process::Command; use std::sync::Arc; use std::time::{Duration, Instant}; -use bollard::Docker; -use fabro_sandbox::{DockerSandbox, DockerSandboxOptions, Sandbox as FabroSandbox, local_sandbox}; +use fabro_sandbox::{DockerSandboxOptions, Sandbox as FabroSandbox, docker_sandbox, local_sandbox}; use sandbox_driver::{ ExecSpec, GrepOptions, Sandbox as DriverSandbox, SandboxProvider, SandboxSource, SandboxSpec, Search, @@ -324,12 +322,13 @@ async fn serve_over_duplex(provider: Arc) -> PluginProvider #[tokio::test(flavor = "multi_thread")] #[ignore = "benchmark: needs a Docker daemon with buildpack-deps:noble and takes about a minute"] async fn agent_tool_call_latency_through_the_driver() { - let Ok(docker) = Docker::connect_with_local_defaults() else { - eprintln!("no Docker daemon; skipping"); - return; - }; - if docker.inspect_image(IMAGE).await.is_err() { - eprintln!("{IMAGE} is not present locally; skipping"); + let image_check = Command::new("docker") + .args(["image", "inspect", IMAGE]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status(); + if !image_check.is_ok_and(|status| status.success()) { + eprintln!("no Docker daemon or {IMAGE} is not present locally; skipping"); return; } let repo = Repository::pack(); @@ -363,8 +362,8 @@ async fn agent_tool_call_latency_through_the_driver() { remote_host.shutdown().await.expect("shutdown"); host.delete().await.expect("host delete"); - // -- Docker, in-process: fabro DockerSandbox vs driver DockerProvider. - let fabro_docker = DockerSandbox::new( + // -- Docker, in-process: fabro's driver-backed sandbox vs the bare driver. + let fabro_docker = docker_sandbox( DockerSandboxOptions { image: IMAGE.to_owned(), auto_pull: false, @@ -378,10 +377,11 @@ async fn agent_tool_call_latency_through_the_driver() { None, None, ) + .await .expect("fabro docker sandbox"); fabro_docker.initialize().await.expect("fabro docker init"); unpack_fabro(&fabro_docker, &repo).await; - rows.extend(bench_fabro("fabro DockerSandbox", &fabro_docker, &repo).await); + rows.extend(bench_fabro("fabro Docker (driver-backed)", &fabro_docker, &repo).await); fabro_docker.cleanup().await.expect("fabro docker cleanup"); let docker_provider = Arc::new(DockerProvider::connect().await.expect("docker connect")); diff --git a/lib/components/fabro-sandbox/tests/error.rs b/lib/components/fabro-sandbox/tests/error.rs index 4d6d786d4..3921e5021 100644 --- a/lib/components/fabro-sandbox/tests/error.rs +++ b/lib/components/fabro-sandbox/tests/error.rs @@ -11,24 +11,3 @@ fn context_error_preserves_source_cause() { "Failed to read file\n caused by: permission denied" ); } - -#[cfg(feature = "docker")] -#[test] -fn docker_image_inspect_error_preserves_source_cause() { - use bollard::errors::Error as BollardError; - - let source = BollardError::DockerResponseServerError { - status_code: 500, - message: "daemon unavailable".to_string(), - }; - - let error = fabro_sandbox::Error::docker_image_inspect("buildpack-deps:noble", source); - - assert_eq!( - error.to_string(), - "Failed to inspect Docker image buildpack-deps:noble" - ); - assert_eq!(error.causes(), vec![ - "Docker responded with status code 500: daemon unavailable" - ]); -} diff --git a/lib/components/fabro-workflow/Cargo.toml b/lib/components/fabro-workflow/Cargo.toml index b326fb139..498f7d44b 100644 --- a/lib/components/fabro-workflow/Cargo.toml +++ b/lib/components/fabro-workflow/Cargo.toml @@ -83,7 +83,7 @@ fabro-acp = { path = "../fabro-acp", features = ["test-support"] } fabro-workflow = { path = ".", features = ["test-support"] } fabro-api = { path = "../../foundation/fabro-api" } fabro-environment = { path = "../fabro-environment" } -fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona", "docker", "test-support"] } +fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona", "test-support"] } fabro-mcp = { path = "../fabro-mcp" } tokio = { workspace = true, features = ["test-util", "macros"] } object_store.workspace = true diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 2c9eade65..2b6f1da82 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -13760,7 +13760,8 @@ async fn asset_collection_docker_sandbox() { ..Default::default() }; let sandbox: Arc = Arc::new( - fabro_agent::DockerSandbox::new(config, None, None, None, None, None, None) + fabro_agent::docker_sandbox(config, None, None, None, None, None, None) + .await .expect("Docker not available"), ); sandbox.initialize().await.expect("Docker init failed");