feat(sandbox): forkd Firecracker microVM provider (rebased onto main, supersedes #517) - #567
Open
zenprocess wants to merge 15 commits into
Open
feat(sandbox): forkd Firecracker microVM provider (rebased onto main, supersedes #517)#567zenprocess wants to merge 15 commits into
zenprocess wants to merge 15 commits into
Conversation
Add a `forkd` feature-gated sandbox provider that runs code inside Firecracker microVMs managed by a forkd controller. Changes: - `fabro-types`: add `SandboxProviderKind::Forkd`; widen `is_clone_based()` - `fabro-sandbox/Cargo.toml`: add `forkd` feature wiring `reqwest` + `rand` - `config.rs`: add `ForkdSnapshotSettings`, `ForkdNetwork`, `ForkdSettings` - `provider.rs`: add `SandboxCreateSpec::Forkd` variant + `pub mod forkd` - `from_environment.rs`: add `forkd_config_from_environment()`; widen `duration_to_minutes_i32` gate to include forkd - `sandbox.rs`: widen `resolve_path` gate to include forkd - `details.rs`: add `forkd::forkd_info_from_name` + `forkd::forkd_details`; add exhaustive `Forkd` arm in `sandbox_details()` match - `lib.rs`: widen `from_environment` gate; add `pub mod forkd` re-export; re-export `ForkdConfig`, `ForkdSandbox`, `ForkdSandboxProvider` - `src/forkd/mod.rs` (new): `ForkdConfig`, `ForkdSandbox` implementing the full `Sandbox` trait — one long-lived VM per sandbox; all file I/O via exec with base64 round-trips for binary safety - `src/provider/forkd.rs` (new): `ForkdSandboxProvider` implementing `SandboxProvider` — list/get/create/delete over forkd REST API FORKD_URL (default http://127.0.0.1:8889) and FORKD_TOKEN (default forkd-local-token) are resolved from environment variables at provider construction time; they are never hardcoded. Cargo unavailable in this environment; code is syntactically careful, mirrors daytona idioms exactly (OnceCell, shell_quote, rand 0.9 API, fabro error types, SandboxEvent variants).
…ermination, hardening Finding 1 (HIGH) — provider/forkd.rs create(): call sandbox.initialize().await? after constructing ForkdSandbox so the microVM is actually provisioned (POST /vms + optional git clone) before SandboxInfo is returned. Previously the VM was never created. Finding 2 (HIGH) — forkd/mod.rs upload_file_from_local(): replace String::from_utf8_lossy (corrupts non-UTF-8 bytes) with a binary-safe path: base64-encode raw bytes via the BASE64 engine and decode inside the VM via `echo <b64> | base64 -d > path`, mirroring how read_file_bytes works in reverse. Finding 3 (MEDIUM) — forkd/mod.rs exec_command(): CommandTermination is now TimedOut when forkd returns no exit_code (killed/OOM/timeout), not the incorrect Exited. Finding 4 (MEDIUM) — forkd/mod.rs glob(): replace the broken `*/pattern` prefix (which requires >=1 intermediate dir and mangles patterns like *.rs) with `-name` for patterns without `/` and `-path <base>/<pattern>` for path-qualified patterns. Hardening: - HTTP client timeout (120 s) + connect_timeout (15 s) on ForkdSandbox; 30 s / 15 s on ForkdSandboxProvider. - Bounded exponential-backoff retry (up to 3 attempts, 250 ms initial, max 10 s) on 5xx responses and connect errors in exec_in_vm, create_vm, delete_vm (mod.rs) and list/get/delete (provider/forkd.rs). - ForkdConfig::Debug is now a manual impl that prints forkd_token as "[redacted]" so the bearer token never appears in tracing output or panic messages. - All error paths use crate::Error::context (no bare unwrap on IO/HTTP). - No hardcoded internal hostnames, IPs, or tokens introduced.
…ovider Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… literals Add the missing `forkd` field to both the `ServerSandboxProvidersLayer` struct (layers/server.rs) and the `ServerSandboxProvidersSettings` literal in `resolve_sandbox` (resolve/server.rs). The `forkd` field was added to `ServerSandboxProvidersSettings` in fabro-types but the config-layer structs and the resolver literal were not updated, causing E0063 (missing field) when building with --features forkd. Both sites now mirror the `daytona` field pattern exactly.
Add missing SandboxProviderKind::Forkd => arms to every match block that covered Daytona but not Forkd, fixing non-exhaustive match errors when building with --features forkd. Files changed: - fabro-install/src/lib.rs: Forkd => false (install wizard does not offer forkd; configured via settings/env) - fabro-sandbox/src/sandbox_spec.rs: add SandboxSpec::Forkd variant + provider()/provider_name()/build()/to_run_sandbox_instance() arms - fabro-sandbox/src/reconnect.rs: #[cfg(feature="forkd")] Forkd reconnect arm + not-enabled fallback - fabro-sandbox/src/terminal.rs: Forkd => error (terminals not supported) - fabro-server/src/server/handler/sandbox.rs: Forkd => conflict (SSH access not supported) - fabro-server/src/run_manifest.rs: Forkd in clone_disabled_for_provider + preflight_sandbox_spec - fabro-workflow/src/operations/start.rs: Forkd => SandboxSpec::Forkd arm
provider/forkd.rs called sandbox.initialize() on a ForkdSandbox value but
did not import the Sandbox trait, causing E0599. Mirror daytona.rs's idiom:
use crate::{Sandbox, details};
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
forkd was registered as a SandboxProviderKind but EnvironmentProvider (the run-level environment selector) had no Forkd variant, so resolve_sandbox_provider() could never route a run to forkd and the SandboxProviderKind::Forkd => SandboxSpec::Forkd arm in start.rs was unreachable. Add the Forkd variant + From<EnvironmentProvider> mapping + is_clone_based, plus the matching arms in the exhaustive matches: default environment TOML (store.rs), capability validation (environment.rs), and capability warnings (run_manifest.rs). Verified: cargo zigbuild --release --target x86_64-unknown-linux-musl -p fabro-cli --features forkd compiles green; binary --version rc=0.
…wire feature end-to-end
Rewrites the forkd client to the actual controller API and makes forkd usable in
a real workflow run (validated live: fabro provisioned a microVM "Sandbox: forkd
ready in 74ms" and executed inside it against a zen-gate-base snapshot).
- forkd/mod.rs: /v1/sandboxes snapshot API — create POST {snapshot_tag} -> id from
response[0] (array), exec POST /{id}/exec {args,timeout_secs} (argv-only; cwd/env
folded into sh -lc), delete /{id}; token-redacted Debug; retry/backoff preserved.
- provider/forkd.rs: list/get/delete -> /v1/sandboxes; create uses server-assigned
sandbox_id() (drops caller-chosen vm_name).
- config.rs/from_environment.rs: snapshot_tag (FORKD_SNAPSHOT_TAG, default zen-gate-base).
- store.rs: seed forkd into seeded_catalog_layer so --environment forkd resolves.
- Cargo: fabro-workflow gains a forkd feature; fabro-cli + fabro-server pass
fabro-workflow/forkd through (previously the worker compiled the disabled arm ->
"Forkd sandbox support is not enabled" at run time).
- specs/forkd-e2e/behavior.feature: BDD contract for create/exec/delete + real run.
Reviewed: 3 adversarial workflow reviewers (8 issues fixed) + ocr (no CRITICAL/HIGH).
Follow-ups (LOW, ocr): FORKD_ENVIRONMENT_ID const; Ready event memory field; install wizard forkd.
…h arms Rebase-time fixes discovered while porting forkd onto 0.290: - fabro-workflow/start.rs and fabro-server/run_manifest.rs imported forkd_config_from_environment unconditionally even though it is gated behind #[cfg(feature = "forkd")] in fabro-sandbox. Since the forkd feature is opt-in (not part of any crate's default features), cargo build --workspace failed with an unresolved-import error. Both imports are now gated the same way resolve_forkd_config already was. - fabro-config/resolve/environment.rs and fabro-server/handler/sandbox.rs had Forkd match arms with bodies identical to an existing arm (Daytona and Local respectively), which clippy::match_same_arms rejects under -D warnings. Merged into combined patterns. - Cargo.lock gains the reqwest dependency edge pulled in by the forkd feature.
cargo dev docker-build hardcoded the cargo zigbuild invocation with no feature flags, so a stock docker-build produced a fabro-cli binary without the forkd provider (forkd is opt-in, not a default feature) -- the same unpushed-binary trap that necessitated this port, now in build-tooling form. Add --features <list> to DockerBuildArgs, threaded through DockerBuildPlan into build_script(), which appends " --features <list>" to the zigbuild command when set. dry_run_lines() picks it up for free since it renders the same build_command().
…nnect config from env Fixes two HIGH findings from the adversarial review of PR #2. HIGH-1: fabro-cli/Cargo.toml:16 — the `forkd` feature omitted `fabro-server/forkd`, so a `--features forkd` build of the `fabro` binary compiled the server's forkd support OFF: the provider was never registered in SandboxProviderRegistry (server.rs:2304) and /preflight always returned "Forkd sandbox support is not enabled" (run_manifest.rs:937). Added "fabro-server/forkd" to the feature list. HIGH-2: fabro-sandbox/src/reconnect.rs — the Forkd reconnect arm built `ForkdSandbox::new(ForkdConfig::default(), ...)`, hardcoding http://127.0.0.1:8889 and the placeholder token "forkd-local-token", which violates ForkdConfig's own contract (forkd/mod.rs) that the URL and token must always come from the environment. On the live hosts FORKD_URL happens to match the default but FORKD_TOKEN is a real secret, so every reconnect-driven operation (session exec, run-file retrieval, sandbox ops) would fail auth against the real controller. Added `ForkdConfig::from_env()` (forkd/mod.rs) as the single place that resolves FORKD_URL/FORKD_TOKEN/FORKD_SNAPSHOT_TAG from the process environment when a full RunEnvironmentSettings isn't available. reconnect.rs now calls it instead of `ForkdConfig::default()`. ForkdSandboxProvider::from_env() (provider/forkd.rs) is refactored to delegate to the same function, removing duplicated env-var resolution and picking up FORKD_SNAPSHOT_TAG resolution it was previously missing. NOTICED (not fixed, out of scope for this fix): `cargo +nightly-2026-04-14 clippy --workspace --all-targets --features fabro-cli/forkd -- -D warnings` surfaces 22 pre-existing clippy findings in fabro-sandbox/src/forkd/mod.rs and provider/forkd.rs, none touched by this commit. They were never caught because the earlier port's clippy acceptance ran with default features only (forkd off). Flagging for a follow-up pass; not fixed here to stay scoped to the two HIGH findings.
The pinned nightly-2026-04-14 rustfmt disagreed with hand-formatted struct field alignment in the forkd sandbox surface (config.rs, details.rs, forkd/mod.rs, provider/forkd.rs, from_environment.rs, lib.rs, provider.rs) and a few call sites touched by the exhaustive match-arm commit (run_manifest.rs, server.rs, start.rs). Upstream's CONTRIBUTING requires a clean `cargo fmt --check --all`; this was carried as known drift on the internal port branch but must be fixed before an upstream contribution. No unrelated files changed -- `cargo +nightly-2026-04-14 fmt --all` touched only the 10 files already present in this branch's diff vs upstream/main.
Upstream's CONTRIBUTING requires a clean clippy -D warnings, and CI runs with the forkd feature enabled for this crate's own tests. The internal port branch only ever ran clippy with default features (forkd off), so this lint debt was invisible there. Mechanical fixes only, no behavior change: - unused import (rand::Rng), unused-self on http_client/resolve_path (both converted to associated fns; call sites updated to Self::...) - reqwest::Client::builder() -> fabro_http::HttpClientBuilder::new() (the approved facade; added dep:fabro-http to the forkd feature) - absolute paths brought into scope via `use` (DEFAULT_SNAPSHOT_TAG, ForkdConfig, tokio::fs) - needless raw-string hashes, format! appended to String -> write!, manual let-else, map().unwrap_or_else() -> map_or_else, manual div_ceil -> .div_ceil(), map_err-for-side-effect -> inspect_err, Duration::from_secs(120) -> from_mins(2), &str -> &'static str on ForkdSandbox::platform() (matching DaytonaSandbox's existing pattern) - #[expect(dead_code, reason = "...")] on ForkdSandbox.run_id, which is stored for parity with Docker/Daytona's run correlation but not yet read (documented reason, not fixed to avoid a non-mechanical wiring decision here) - #[allow(clippy::cast_possible_truncation, reason = "...")] on the bytes->MiB memory conversion, matching the existing convention in daytona/details.rs Reformatted with the pinned nightly-2026-04-14 rustfmt (also required by upstream's clean `fmt --check --all`).
The environments table's provider CHECK from 2026063002_environments.sql
only permits ('local','docker','daytona'), so importing an environment
with provider = "forkd" fails the constraint (SQLite code 275) and the
server crash-loops at boot. sqlx::migrate! checksums applied migrations,
so 2026063002 must not be edited — add a new migration instead.
SQLite cannot ALTER a CHECK in place, so 2026071001_environments_forkd.sql
uses the standard table-recreate pattern: create environments_new with
the widened CHECK (adds 'forkd'), INSERT ... SELECT all rows across, DROP
the old table, ALTER ... RENAME. 2026063002 defines no indexes or triggers
on the table, so none need to be re-created.
Tests: environments_schema_accepts_forkd_provider asserts a provider='forkd'
row now inserts and that provider='bogus' is still rejected after the
migration.
Author
|
To unblock the architecture conversation from #517 (the "More soon" follow-up from @brynary never landed in the thread), I've opened #583 — "Sandbox provider architecture preconditions before landing forkd (re #517/#567)". The new issue lists what #567 already conforms to (existing Pinging here for visibility; discussion should live in #583 so it stays on the record. |
Collaborator
|
Hey @zenprocess , thanks for this! With the rise of all these sandbox/vm providers, @brynary and I have been talking about restructuring the way we handle and connect to sandboxes in more of a plugin style system so that we can support any of the services that someone might use. We have a little bit more to discuss here and a bit of work to do before we can land it. |
Author
|
Thanks @swerner, @brynary — a plugin-style provider system is exactly the right call, and it's the abstraction that lets forkd land cleanly. Firecracker microVMs are a genuinely different shape from container providers (snapshot/restore, hermetic boot, per-run isolation), so if it's useful, the forkd provider here can serve as a reference implementation / stress-test for the interface — it should surface any assumptions that only hold for container-based sandboxes. Would you be able to share your reference architecture or current design thinking for the plugin system, even rough? We'd much rather align forkd to your direction and contribute the work than diverge — happy to prototype the provider interface against it, or take on a chunk of the implementation, whichever is most useful to you. |
Member
|
@zenprocess thanks for your patience on this. Here is an initial sketch of how we are considering a plugin system for sandbox providers based on JSON-RPC: https://quarry.lithos.computer/tmp/3cda04aa788840cdb46bcbefb5f60fd4 Feedback welcome! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds forkd (Firecracker microVM controller) as a sandbox provider, selectable per run via a named environment (
provider = "forkd"). It supersedes draft #517 with the same provider rebased onto currentmain— #517's branch had fallen 50+ commits behind and its diverged/unknownmergeable state made it a poor review target, so this is a clean resubmission rather than a force-push over the existing thread.Re: the architecture note on #517
@brynary flagged in #517 that the review surfaced "pre-existing architectural design issues that we may want to resolve before adding the next sandbox provider," with details to follow. We'd genuinely like to align on that before (or as part of) landing this — if there's a refactor you want as a precondition (provider registry shape,
Sandboxtrait split, clone-source contract, anything else), name it and we're happy to take it on or adapt this PR to it. Happy to move that conversation to an Issue/Discussion if that's the better venue per CONTRIBUTING's "align on approach first" guidance for larger features.In the meantime, this PR deliberately conforms to the existing contracts rather than inventing new ones:
Sandboxtrait and provider registry as-is; no trait changes.is_clone_based(), run-manifest GitHub origin metadata,skip_clonesemantics) exactly as Docker/Daytona do.shell_quote(); the one env-var export path is guarded by an identifier-only allowlist.FORKD_URL/FORKD_TOKEN/FORKD_SNAPSHOT_TAGfrom the environment, token redacted inDebug.forkdfeature, default off): default builds compile byte-compatible withmain; every always-compiled match overSandboxProviderKindpairs a gated arm with a graceful-error arm.Delta over #517's original 9 commits
main(no longer 50+ commits behind; adapts to the clone-based provider and config changes that landed since).#[cfg(feature = "forkd")]-gated imports that were unconditional;fabro-cli'sforkdfeature now propagates tofabro-serversofabro serveractually registers the provider.ForkdConfigis resolved from the environment on reconnect (wasDefault, i.e. hardcoded localhost + dev token).environments.providerCHECK to includeforkd(table-recreate; the applied migration is untouched for checksum integrity), with accept/reject tests.cargo dev docker-build --features <list>passthrough so a release image build can enable the feature explicitly.--features forkd -D warnings(22 findings fixed).Testing
cargo build --workspace(default features) — exit 0cargo check -p fabro-cli --features forkd— exit 0cargo nextest run -p fabro-sandbox -p fabro-server -p fabro-db— pass (incl. new forkd CHECK-constraint tests)cargo +nightly fmt --check --all— cleancargo clippy --workspace --all-targets -- -D warnings— clean, both with and without--features fabro-cli/forkd/v1/sandboxesAPI): create-from-snapshot, exec, file upload, teardown.fabro serverboots clean at 0.290, theforkdprovider registers, a[run.environment] id = "forkd"environment resolves, and runs dispatch through it to the live forkd controller.Happy to iterate — and again, if the architectural preconditions from #517 are written up anywhere, point us at them and we'll align.