scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-10 22:43:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix(sandbox): simplify Bash contract implementation

Browse source
This commit is contained in:
Bryan Helmkamp 2026-07-24 21:55:36 -04:00
parent 3606ba6a0f
commit 1ca9fe977d
No known key found for this signature in database
14 changed files with 271 additions and 155 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -2539,6 +2539,7 @@ dependencies = [
"serde",
"serde_json",
"strsim 0.11.1",
"strum 0.28.0",
"temp-env",
"tempfile",
"thiserror 2.0.18",

2
docs/public/agents/mcp.mdx
View file

@ -234,7 +234,7 @@ tool_timeout = "2m"
The sandbox transport requires a remote sandbox provider (Daytona) that supports preview URLs. During session initialization, Fabro:
1. Launches the server inside the sandbox with `setsid bash -c` to fully detach the process
1. Launches the server inside the sandbox with `setsid "$BASH" -c` to fully detach the process while reusing the provider-selected Bash
2. Polls until the server is listening on the configured port (up to 30 seconds)
3. Obtains an authenticated preview URL from the sandbox provider
4. Connects to the server over HTTP using the preview URL

2
docs/public/changelog/2026-07-24.mdx
View file

@ -15,7 +15,7 @@ date: "2026-07-24"
Every sandbox now evaluates command strings as **non-login Bash** — `bash -c`, with no `sh` fallback and no ambient provider shell. Previously the three backends disagreed: Daytona ran commands through `sh`, and Docker's streaming, stdio, and setup paths used a login shell. Bash-only syntax such as `[[ ... ]]` and arrays now behaves identically across local, Docker, and Daytona, through both buffered and streaming execution.
The `shell` tool's name and JSON schema are unchanged; only its description became explicit that `command` is Bash source. Fabro still adds no shell options of its own — there is no implicit `errexit` or `pipefail`, so `false | true` still succeeds. A workflow that wants different semantics writes them into the command (`sh -c ...`, a `#!/bin/sh` shebang, an explicit `set -o pipefail`).
The `shell` tool's name, argument names and types, and required fields are unchanged; its descriptions now make explicit that `command` is Bash source. Fabro still adds no shell options of its own — there is no implicit `errexit` or `pipefail`, so `false | true` still succeeds. A workflow that wants different semantics writes them into the command (`sh -c ...`, a `#!/bin/sh` shebang, an explicit `set -o pipefail`).
Both fresh initialization and resume now verify Bash before reporting the sandbox usable, so a missing or non-Bash interpreter fails at the lifecycle boundary with provider-specific remediation instead of on the first command. Docker and Daytona require `/bin/bash`; local sandboxes resolve `bash` through the worker's `PATH`, which keeps NixOS working.

22
lib/components/fabro-agent/src/session.rs
View file

@ -20,6 +20,7 @@ use fabro_types::{
AgentToolSummary, PermissionLevel, Principal, SessionMessage, SessionRecord,
StageContextWindowProjection, SteeringMessage,
};
use fabro_util::shell;
use futures::StreamExt;
use tokio::sync::{Notify, broadcast};
use tokio::time;
@ -2079,17 +2080,14 @@ const fn is_auth_error(err: &LlmError) -> bool {
/// `setsid` fully detaches the server so Daytona's exec doesn't block on it.
/// The inner command is shell-quoted for the wrapper so a single quote or
/// metacharacter in any argv element can't break out, and the wrapper itself is
/// `bash -c` because the sandbox evaluates this string as non-login Bash.
/// the current `$BASH` because the sandbox evaluates this string as non-login
/// Bash and may resolve that executable outside `/bin` (for example on NixOS).
fn sandbox_mcp_launch_script(command: &[String]) -> String {
let cmd_str = command
.iter()
.map(|arg| fabro_sandbox::shell_quote(arg))
.collect::<Vec<_>>()
.join(" ");
let cmd_str = shell::shell_join(command);
let inner = format!("{cmd_str} > /tmp/mcp_server_stdout.log 2>/tmp/mcp_server_stderr.log");
format!(
"setsid bash -c {quoted} </dev/null >/dev/null 2>&1 &\necho $!",
quoted = fabro_sandbox::shell_quote(&inner)
"setsid \"$BASH\" -c {quoted} </dev/null >/dev/null 2>&1 &\necho $!",
quoted = shell::shell_quote(&inner)
)
}
@ -2137,7 +2135,7 @@ mod tests {
#[test]
fn sandbox_mcp_launch_wrapper_uses_bash() {
// The sandbox evaluates this string as non-login Bash, so the detached
// wrapper must name bash rather than sh.
// wrapper reuses the executable selected by the provider.
let script = sandbox_mcp_launch_script(&[
"npx".to_string(),
"@playwright/mcp@latest".to_string(),
@ -2146,8 +2144,8 @@ mod tests {
]);
assert!(
script.starts_with("setsid bash -c "),
"launch wrapper should detach through bash: {script}"
script.starts_with("setsid \"$BASH\" -c "),
"launch wrapper should detach through the provider-selected Bash: {script}"
);
assert!(
script.ends_with(" </dev/null >/dev/null 2>&1 &\necho $!"),
@ -2171,7 +2169,7 @@ mod tests {
]);
let wrapper_argument = script
.strip_prefix("setsid bash -c ")
.strip_prefix("setsid \"$BASH\" -c ")
.and_then(|rest| rest.strip_suffix(" </dev/null >/dev/null 2>&1 &\necho $!"))
.expect("launch wrapper should have the canonical shape");

85
lib/components/fabro-sandbox/src/daytona/mod.rs
View file

@ -27,19 +27,14 @@ use tokio_util::sync::CancellationToken;
use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason};
use crate::redact::redact_auth_url;
use crate::sandbox::{
BASH_PROBE_SCRIPT, RefreshOutcome, bash_probe_passed, optional_timeout, resolve_path,
BASH_PROBE_SCRIPT, BASH_PROBE_TIMEOUT_MS, REMOTE_BASH, RefreshOutcome, optional_timeout,
resolve_path, validate_bash_probe,
};
use crate::{
CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox,
SandboxEvent, SandboxEventCallback, StdioProcess, glob_match, managed_labels, shell_quote,
};
/// Bash executable Daytona sandboxes require, on both sides of the transport.
///
/// Daytona snapshots are Linux images Fabro documents a requirement for, so
/// the path is known rather than resolved, and there is no `sh` fallback.
pub(crate) const DAYTONA_BASH: &str = "/bin/bash";
/// Remediation shown when a Daytona sandbox has no usable Bash.
const DAYTONA_BASH_REMEDIATION: &str = "Daytona sandboxes require /bin/bash for every command, with no `sh` fallback. Use the \
built-in Daytona snapshot, or a custom snapshot whose Dockerfile installs bash.";
@ -504,7 +499,8 @@ impl DaytonaSandbox {
/// again after a reconnected sandbox starts, so a snapshot without Bash
/// fails at the lifecycle boundary rather than on some later command.
async fn probe_bash(sandbox: &daytona_sdk::Sandbox) -> crate::Result<()> {
let execution = async {
let start = Instant::now();
let execution = time::timeout(Duration::from_millis(BASH_PROBE_TIMEOUT_MS), async {
let process_svc = sandbox
.process()
.await
@ -514,14 +510,27 @@ impl DaytonaSandbox {
&wrap_bash_command(BASH_PROBE_SCRIPT),
daytona_sdk::ExecuteCommandOptions {
cwd: Some("/".to_string()),
timeout: Some(Duration::from_millis(BASH_PROBE_TIMEOUT_MS)),
..Default::default()
},
)
.await
.map_err(|e| crate::Error::context("Failed to run Daytona Bash check", e))?;
Ok((result.exit_code, result.result))
}
Ok(ExecResult {
stdout: result.result,
stderr: String::new(),
exit_code: Some(result.exit_code),
termination: CommandTermination::Exited,
duration_ms: elapsed_ms(start),
})
})
.await;
let execution = match execution {
Ok(result) => result,
Err(_) => Err(crate::Error::message(format!(
"Daytona Bash check timed out after {BASH_PROBE_TIMEOUT_MS}ms"
))),
};
daytona_bash_probe_outcome(execution)
}
@ -1720,7 +1729,7 @@ impl Sandbox for DaytonaSandbox {
let mut session = DaytonaSession::create(sandbox).await?;
let session_command =
wrap_bash_command(&build_bash_session_script(command, &cwd, env_vars));
wrap_bash_session_script(&build_bash_session_script(command, &cwd, env_vars));
let session_exec = match session.execute(&session_command, true, true).await {
Ok(result) => result,
Err(err) => {
@ -2291,8 +2300,7 @@ fn missing_log_suffix_offset(seen: &[u8], final_bytes: &[u8]) -> usize {
/// Build the inner Bash script a session command evaluates.
///
/// The result is Bash source, not something Daytona can exec directly — it is
/// passed through [`wrap_bash_command`] before reaching the toolbox, exactly
/// like the non-streaming path.
/// passed through [`wrap_bash_session_script`] before reaching the toolbox.
fn build_bash_session_script(
command: &str,
cwd: &str,
@ -2322,14 +2330,10 @@ fn build_bash_session_script(
///
/// A failure to run the probe at all, a nonzero exit, and a zero exit without
/// the marker are all probe failures, and all carry the snapshot remediation.
fn daytona_bash_probe_outcome(execution: crate::Result<(i32, String)>) -> crate::Result<()> {
fn daytona_bash_probe_outcome(execution: crate::Result<ExecResult>) -> crate::Result<()> {
match execution {
Err(err) => Err(crate::Error::context(DAYTONA_BASH_REMEDIATION, err)),
Ok((exit_code, output)) if bash_probe_passed(Some(exit_code), &output) => Ok(()),
Ok((exit_code, output)) => Err(crate::Error::message(format!(
"Daytona sandbox Bash check failed (exit {exit_code}). {DAYTONA_BASH_REMEDIATION} {}",
output.trim()
))),
Ok(result) => validate_bash_probe(result, DAYTONA_BASH_REMEDIATION),
}
}
@ -2356,7 +2360,17 @@ fn wrap_bash_command(command: &str) -> String {
use base64::Engine;
use base64::engine::general_purpose::STANDARD;
let encoded = STANDARD.encode(command);
format!("{DAYTONA_BASH} -c \"echo '{encoded}' | base64 -d | {DAYTONA_BASH}\"")
format!("{REMOTE_BASH} -c \"echo '{encoded}' | base64 -d | {REMOTE_BASH}\"")
}
/// Enter the canonical Bash interpreter from a Daytona streaming session.
///
/// Session commands already pass through the provider's shell parser, so an
/// audited shell-quoted argument avoids the direct-exec path's base64 process
/// and second Bash while keeping caller source inert until `/bin/bash -c`
/// evaluates it.
fn wrap_bash_session_script(script: &str) -> String {
format!("exec {REMOTE_BASH} -c {}", shell_quote(script))
}
#[cfg(test)]
@ -2961,25 +2975,40 @@ mod tests {
#[test]
fn streaming_session_script_reaches_bash_through_the_canonical_wrapper() {
let script = build_bash_session_script("[[ -d / ]] && echo ok", "/tmp", None);
let wrapped = wrap_bash_session_script(&script);
assert_eq!(
decode_wrapped_command(&wrap_bash_command(&script)),
script,
"the streaming path must use the same wrapper as exec_command"
wrapped,
format!("exec /bin/bash -c {}", shell_quote(&script)),
"the streaming path must enter the canonical Bash exactly once"
);
assert!(!wrapped.contains("base64"));
}
fn bash_probe_result(exit_code: i32, stdout: impl Into<String>) -> ExecResult {
ExecResult {
stdout: stdout.into(),
stderr: String::new(),
exit_code: Some(exit_code),
termination: CommandTermination::Exited,
duration_ms: 1,
}
}
#[test]
fn bash_probe_outcome_accepts_a_marked_zero_exit() {
assert!(daytona_bash_probe_outcome(Ok((0, format!("{BASH_PROBE_MARKER}\n")))).is_ok());
assert!(
daytona_bash_probe_outcome(Ok(bash_probe_result(0, format!("{BASH_PROBE_MARKER}\n"))))
.is_ok()
);
}
#[test]
fn bash_probe_outcome_rejects_failures_with_snapshot_remediation() {
let failures = [
Err(crate::Error::message("connection reset")),
Ok((1, "bash: not found".to_string())),
Ok((0, "ready".to_string())),
Ok(bash_probe_result(1, "bash: not found")),
Ok(bash_probe_result(0, "ready")),
];
for failure in failures {
@ -2991,6 +3020,10 @@ mod tests {
err.contains("/bin/bash") && err.contains("snapshot"),
"probe failure should carry the Daytona snapshot remediation: {err}"
);
assert!(
!err.contains("bash: not found"),
"raw process output must not enter lifecycle errors: {err}"
);
}
}

62
lib/components/fabro-sandbox/src/docker.rs
View file

@ -29,8 +29,8 @@ use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason};
use crate::managed_labels::{self, MANAGED_LABEL, RUN_ID_LABEL};
use crate::redact::redact_auth_url;
use crate::sandbox::{
BASH_PROBE_SCRIPT, RefreshOutcome, StdioProcessControl, bash_probe_passed, optional_timeout,
resolve_path,
BASH_PROBE_SCRIPT, BASH_PROBE_TIMEOUT_MS, REMOTE_BASH, RefreshOutcome, StdioProcessControl,
optional_timeout, resolve_path, validate_bash_probe,
};
use crate::{
CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
@ -39,12 +39,8 @@ use crate::{
shell_quote,
};
/// Bash executable Docker sandboxes require.
///
/// Unlike the local sandbox there is no `PATH` resolution here: the remote
/// filesystem is a Linux image Fabro documents a requirement for, and there is
/// deliberately no `sh` fallback when it is missing.
pub(crate) const DOCKER_BASH: &str = "/bin/bash";
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.";
pub(crate) const WORKING_DIRECTORY: &str = "/workspace";
pub(crate) const REPOS_ROOT: &str = "/repos";
@ -420,7 +416,7 @@ impl DockerSandbox {
let env: Option<Vec<String>> =
env_vars.map(|vars| vars.iter().map(|(k, v)| format!("{k}={v}")).collect());
let cmd = vec![
DOCKER_BASH.to_string(),
REMOTE_BASH.to_string(),
"-c".to_string(),
command.to_string(),
];
@ -481,7 +477,7 @@ impl DockerSandbox {
let (stop_file, pid_file) = docker_exec_control_paths();
let controlled_command = docker_controlled_shell_command(command, &stop_file, &pid_file);
let cmd = vec![
DOCKER_BASH.to_string(),
REMOTE_BASH.to_string(),
"-c".to_string(),
controlled_command,
];
@ -607,27 +603,17 @@ impl DockerSandbox {
/// resumed container cannot pass startup and then fail on its first
/// command.
async fn probe_bash(&self, working_dir: Option<&str>) -> crate::Result<()> {
let (stdout, stderr, exit_code) = self
.docker_exec(
vec![
DOCKER_BASH.to_string(),
"-c".to_string(),
BASH_PROBE_SCRIPT.to_string(),
],
working_dir,
let result = self
.docker_exec_shell(
BASH_PROBE_SCRIPT,
BASH_PROBE_TIMEOUT_MS,
Some(working_dir.unwrap_or("/")),
None,
None,
)
.await?;
if bash_probe_passed(Some(exit_code), &stdout) {
return Ok(());
}
Err(crate::Error::message(format!(
"Docker container Bash check failed (exit {exit_code}). Docker sandboxes require \
{DOCKER_BASH} for every command, with no `sh` fallback; use an image with bash and \
git, such as buildpack-deps:noble. {stderr}"
)))
.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<()> {
@ -910,7 +896,7 @@ wait \"$watcher\" 2>/dev/null || true; \
rm -f \"$stop_file\" \"$pid_file\"; \
exit \"$status\"\
",
bash = DOCKER_BASH,
bash = REMOTE_BASH,
stop_file = shell_quote(stop_file),
pid_file = shell_quote(pid_file),
command = shell_quote(command),
@ -930,7 +916,7 @@ fn docker_stdio_exec_options(
attach_stdout: Some(true),
attach_stderr: Some(true),
tty: Some(false),
cmd: Some(vec![DOCKER_BASH.to_string(), "-c".to_string(), command]),
cmd: Some(vec![REMOTE_BASH.to_string(), "-c".to_string(), command]),
working_dir: Some(working_dir),
env,
..Default::default()
@ -967,7 +953,7 @@ async fn create_and_start_exec(
fn docker_stop_request_exec_options(stop_file: &str) -> CreateExecOptions<String> {
CreateExecOptions {
cmd: Some(vec![
DOCKER_BASH.to_string(),
REMOTE_BASH.to_string(),
"-c".to_string(),
format!("touch {}", shell_quote(stop_file)),
]),
@ -1178,7 +1164,7 @@ fn container_config(config: &DockerSandboxOptions, run_id: Option<&RunId>) -> Co
Config {
image: Some(config.image.clone()),
cmd: Some(vec![
DOCKER_BASH.to_string(),
REMOTE_BASH.to_string(),
"-c".to_string(),
format!(
"mkdir -p {} && sleep infinity",
@ -1233,10 +1219,8 @@ fn docker_not_modified(error: &DockerError) -> bool {
})
}
fn bash_remediation(error: &DockerError, image: &str) -> String {
format!(
"Failed to start Docker container from image '{image}': {error}. Docker sandboxes require {DOCKER_BASH} for every command, with no `sh` fallback; use an image with bash and git, such as buildpack-deps:noble."
)
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]) -> crate::Result<Vec<u8>> {
@ -1351,7 +1335,7 @@ impl Sandbox for DockerSandbox {
.start_container(&id, None::<StartContainerOptions<String>>)
.await
.map_err(|e| {
let err = crate::Error::context(bash_remediation(&e, &self.config.image), e);
let err = crate::Error::context(bash_remediation(&self.config.image), e);
self.fail_init(init_start, err)
})?;
@ -2023,7 +2007,7 @@ mod tests {
use tokio::process::Command;
use super::*;
use crate::sandbox::BASH_PROBE_MARKER;
use crate::sandbox::{BASH_PROBE_MARKER, bash_probe_passed};
#[test]
fn per_run_container_idle_command_uses_non_login_bash() {

99
lib/components/fabro-sandbox/src/local.rs
View file

@ -12,7 +12,10 @@ use tokio::task::spawn_blocking;
use tokio::{fs, time};
use tokio_util::sync::CancellationToken;
use crate::sandbox::{BASH_PROBE_SCRIPT, StdioProcessControl, bash_probe_passed, optional_timeout};
use crate::sandbox::{
BASH_PROBE_SCRIPT, BASH_PROBE_TIMEOUT_MS, StdioProcessControl, optional_timeout,
validate_bash_probe,
};
use crate::{
CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector,
@ -100,30 +103,20 @@ impl LocalSandbox {
/// it.
async fn probe_bash(&self) -> crate::Result<()> {
let bash = self.bash()?;
let output = Command::new(&bash)
.arg("-c")
.arg(BASH_PROBE_SCRIPT)
.env_clear()
.envs(filtered_env_vars(None, ExplicitEnvPolicy::FilterSensitive))
.output()
let remediation = format!(
"{} is not usable as non-login Bash. {LOCAL_BASH_REMEDIATION}",
bash.display()
);
let result = self
.exec_command(BASH_PROBE_SCRIPT, BASH_PROBE_TIMEOUT_MS, None, None, None)
.await
.map_err(|e| {
.map_err(|err| {
crate::Error::context(
format!("{LOCAL_BASH_REMEDIATION} Failed to run {}", bash.display()),
e,
format!("Failed to run the local Bash check. {remediation}"),
err,
)
})?;
let stdout = String::from_utf8_lossy(&output.stdout);
if bash_probe_passed(output.status.code(), &stdout) {
return Ok(());
}
Err(crate::Error::message(format!(
"{LOCAL_BASH_REMEDIATION} {} is not usable as non-login Bash: {}",
bash.display(),
String::from_utf8_lossy(&output.stderr).trim()
)))
validate_bash_probe(result, remediation)
}
pub fn set_event_callback(&mut self, cb: SandboxEventCallback) {
@ -451,6 +444,7 @@ impl Sandbox for LocalSandbox {
.current_dir(&effective_dir)
.env_clear()
.envs(filtered_env)
.kill_on_drop(true)
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped());
@ -528,6 +522,7 @@ impl Sandbox for LocalSandbox {
.current_dir(&effective_dir)
.env_clear()
.envs(filtered_env)
.kill_on_drop(true)
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped());
@ -823,7 +818,23 @@ impl Sandbox for LocalSandbox {
/// Bash it initialized with; there is deliberately no fallback interpreter
/// when that happens.
async fn start(&self) -> crate::Result<()> {
self.probe_bash().await
self.emit(SandboxEvent::StartStarted {
provider: "local".into(),
});
let start = Instant::now();
let result = self.probe_bash().await;
match &result {
Ok(()) => self.emit(SandboxEvent::StartCompleted {
provider: "local".into(),
duration_ms: elapsed_ms(start),
}),
Err(err) => self.emit(SandboxEvent::StartFailed {
provider: "local".into(),
error: err.to_string(),
causes: err.causes(),
}),
}
result
}
async fn git_push_ref(&self, refspec: &str) -> crate::Result<()> {
@ -1405,18 +1416,56 @@ mod tests {
#[tokio::test]
async fn start_repeats_the_bash_probe_on_resume() {
let dir = temp_dir();
use std::sync::Mutex;
LocalSandbox::new(dir.clone())
let dir = temp_dir();
let successful_events: Arc<Mutex<Vec<SandboxEvent>>> = Arc::new(Mutex::new(Vec::new()));
let successful_events_clone = Arc::clone(&successful_events);
let mut available = LocalSandbox::new(dir.clone());
available.set_event_callback(Arc::new(move |event| {
successful_events_clone.lock().unwrap().push(event);
}));
available
.start()
.await
.expect("resume should succeed when Bash is present");
LocalSandbox::with_bash_executable(dir.clone(), BashExecutable::Unavailable)
let failed_events: Arc<Mutex<Vec<SandboxEvent>>> = Arc::new(Mutex::new(Vec::new()));
let failed_events_clone = Arc::clone(&failed_events);
let mut unavailable =
LocalSandbox::with_bash_executable(dir.clone(), BashExecutable::Unavailable);
unavailable.set_event_callback(Arc::new(move |event| {
failed_events_clone.lock().unwrap().push(event);
}));
unavailable
.start()
.await
.expect_err("resume should fail when Bash disappeared between runs");
let successful_events = successful_events.lock().unwrap();
assert!(matches!(
&successful_events[..],
[
SandboxEvent::StartStarted { provider: started },
SandboxEvent::StartCompleted {
provider: completed,
..
}
] if started == "local" && completed == "local"
));
let failed_events = failed_events.lock().unwrap();
assert!(matches!(
&failed_events[..],
[
SandboxEvent::StartStarted { provider: started },
SandboxEvent::StartFailed {
provider: failed,
..
}
] if started == "local" && failed == "local"
));
std::fs::remove_dir_all(&dir).unwrap();
}

72
lib/components/fabro-sandbox/src/sandbox.rs
View file

@ -21,6 +21,13 @@ const GIT: &str = "git -c maintenance.auto=0 -c gc.auto=0";
pub const DEFAULT_EXEC_OUTPUT_TAIL_BYTES: usize = 8 * 1024;
/// Maximum time a sandbox lifecycle check may spend proving Bash is usable.
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";
/// Marker a successful [`BASH_PROBE_SCRIPT`] run prints on stdout.
///
/// Providers validate the marker rather than trusting a zero exit: an image
@ -51,9 +58,29 @@ printf '%s\n' 'fabro-bash-ready'"#;
/// Whether a [`BASH_PROBE_SCRIPT`] run succeeded.
///
/// A zero exit without the marker is not a successful probe.
/// A zero exit without exactly the marker is not a successful probe.
pub(crate) fn bash_probe_passed(exit_code: Option<i32>, stdout: &str) -> bool {
exit_code == Some(0) && stdout.contains(BASH_PROBE_MARKER)
exit_code == Some(0) && stdout.trim() == BASH_PROBE_MARKER
}
/// Validate a completed Bash probe without flattening its raw output into an
/// error message.
///
/// [`Error::Exec`](crate::Error::Exec) retains stdout/stderr for the existing
/// redacted-tail diagnostics while its display form exposes only bounded,
/// classified metadata safe for lifecycle events and tracing.
pub(crate) fn validate_bash_probe(
result: ExecResult,
remediation: impl Into<String>,
) -> crate::Result<()> {
if result.is_success() && bash_probe_passed(result.exit_code, &result.stdout) {
return Ok(());
}
Err(crate::Error::context(
remediation,
result.into_exec_error("Sandbox Bash probe"),
))
}
/// Sleep for `timeout_ms` if `Some`, otherwise never resolves. Used by
@ -1572,6 +1599,7 @@ mod tests {
let output = Command::new(program)
.args(args)
.arg(BASH_PROBE_SCRIPT)
.env_remove("BASH_ENV")
.output()
.await
.expect("probe should run");
@ -1584,7 +1612,7 @@ mod tests {
let (code, stdout) = run("bash", &["-c"]).await;
assert!(bash_probe_passed(code, &stdout), "non-login bash: {stdout}");
let (code, stdout) = run("bash", &["-lc"]).await;
let (code, stdout) = run("bash", &["--noprofile", "-lc"]).await;
assert!(
!bash_probe_passed(code, &stdout),
"a login shell must fail the probe: {stdout}"
@ -1600,6 +1628,44 @@ mod tests {
);
}
#[test]
fn bash_probe_requires_the_exact_marker_output() {
assert!(bash_probe_passed(
Some(0),
&format!(" {BASH_PROBE_MARKER}\n")
));
assert!(!bash_probe_passed(
Some(0),
&format!("prefix-{BASH_PROBE_MARKER}-suffix")
));
assert!(!bash_probe_passed(
Some(0),
&format!("{BASH_PROBE_MARKER}\nunexpected output")
));
}
#[test]
fn bash_probe_failure_keeps_raw_output_out_of_the_error_chain() {
let err = validate_bash_probe(
ExecResult {
stdout: String::new(),
stderr: "raw-probe-output".to_string(),
exit_code: Some(1),
termination: CommandTermination::Exited,
duration_ms: 1,
},
"Install Bash",
)
.expect_err("failed probe should return remediation");
assert!(!err.display_with_causes().contains("raw-probe-output"));
assert_eq!(
err.default_redacted_output_tail()
.and_then(|tail| tail.stderr),
Some("raw-probe-output".to_string())
);
}
#[expect(
clippy::disallowed_methods,
reason = "unit test performs a small synchronous source scan of local Rust files"

12
lib/components/fabro-sandbox/tests/daytona_streaming_live.rs
View file

@ -98,17 +98,7 @@ mod daytona_streaming_live {
"exec_command should report the Bash-only result",
)?;
let chunks = Arc::new(Mutex::new(Vec::new()));
let streaming = sandbox
.exec_command_streaming(
command,
Some(30_000),
None,
None,
None,
capture_callback(Arc::clone(&chunks)),
)
.await?;
let (streaming, _) = run_captured(&sandbox, command, 30_000, None).await?;
ensure_eq(
&streaming.result.exit_code,
&Some(0),

37
lib/components/fabro-sandbox/tests/docker_streaming.rs
View file

@ -6,6 +6,16 @@ use bollard::Docker;
use fabro_sandbox::{CommandOutputCallback, DockerSandbox, DockerSandboxOptions, Sandbox};
use tokio::sync::Mutex;
fn capture_bytes(chunks: Arc<Mutex<Vec<u8>>>) -> CommandOutputCallback {
Arc::new(move |_stream, bytes| {
let chunks = Arc::clone(&chunks);
Box::pin(async move {
chunks.lock().await.extend(bytes);
Ok(())
})
})
}
#[tokio::test]
#[ignore = "requires real Docker container lifecycle; run explicitly when changing Docker exec integration"]
async fn streaming_timeout_terminates_docker_exec_before_returning() {
@ -36,14 +46,6 @@ async fn streaming_timeout_terminates_docker_exec_before_returning() {
.expect("docker sandbox should initialize");
let chunks = Arc::new(Mutex::new(Vec::new()));
let callback_chunks = Arc::clone(&chunks);
let callback: CommandOutputCallback = Arc::new(move |_stream, bytes| {
let callback_chunks = Arc::clone(&callback_chunks);
Box::pin(async move {
callback_chunks.lock().await.extend(bytes);
Ok(())
})
});
let marker = "fabro_streaming_timeout_sentinel";
let result = sandbox
@ -53,7 +55,7 @@ async fn streaming_timeout_terminates_docker_exec_before_returning() {
None,
None,
None,
callback,
capture_bytes(Arc::clone(&chunks)),
)
.await
.expect("streaming command should return a timeout result");
@ -193,16 +195,15 @@ async fn docker_runs_bash_only_syntax_through_both_command_paths() {
.expect("non-streaming command should run");
let chunks = Arc::new(Mutex::new(Vec::new()));
let callback_chunks = Arc::clone(&chunks);
let callback: CommandOutputCallback = Arc::new(move |_stream, bytes| {
let callback_chunks = Arc::clone(&callback_chunks);
Box::pin(async move {
callback_chunks.lock().await.extend(bytes);
Ok(())
})
});
let streaming = sandbox
.exec_command_streaming(command, Some(10_000), None, None, None, callback)
.exec_command_streaming(
command,
Some(10_000),
None,
None,
None,
capture_bytes(Arc::clone(&chunks)),
)
.await
.expect("streaming command should run");

9
lib/components/fabro-workflow/tests/it/daytona_integration.rs
View file

@ -28,6 +28,7 @@ use fabro_sandbox::daytona::{DaytonaConfig, DaytonaSandbox};
use fabro_static::EnvVars;
use fabro_store::{ArtifactKey, ArtifactStore, Database};
use fabro_types::{RunId, StageId, WorkflowSettings};
use fabro_util::shell;
use fabro_workflow::artifact::sync_artifacts_to_env;
use fabro_workflow::context::Context;
use fabro_workflow::error::Error;
@ -1794,10 +1795,12 @@ async fn daytona_playwright_mcp_sandbox_transport() {
..
} => {
let (url, headers) = {
let cmd_str = command.join(" ");
let cmd_str = shell::shell_join(command);
let inner =
format!("{cmd_str} > /tmp/mcp_server_stdout.log 2>/tmp/mcp_server_stderr.log");
let launch_script = format!(
"setsid bash -c '{cmd_str} > /tmp/mcp_server_stdout.log 2>/tmp/mcp_server_stderr.log' \
</dev/null >/dev/null 2>&1 &\necho $!"
"setsid \"$BASH\" -c {} </dev/null >/dev/null 2>&1 &\necho $!",
shell::shell_quote(&inner)
);
let launch_result = sandbox
.exec_command(&launch_script, 30_000, None, None, None)

1
lib/foundation/fabro-config/Cargo.toml
View file

@ -31,6 +31,7 @@ dirs.workspace = true
ipnet = "2.11.0"
serde.workspace = true
serde_json.workspace = true
strum.workspace = true
strsim = "0.11"
tempfile = "3"
toml.workspace = true

20
lib/foundation/fabro-config/src/resolve/run.rs
View file

@ -490,11 +490,8 @@ fn resolve_mcp_command(
interpreter: ScriptInterpreter,
) -> Vec<String> {
if let Some(script) = script {
return vec![
interpreter.executable().to_string(),
"-c".to_string(),
script.as_source(),
];
let executable: &'static str = interpreter.into();
return vec![executable.to_string(), "-c".to_string(), script.as_source()];
}
command
.map(|command| command.iter().map(InterpString::as_source).collect())
@ -506,24 +503,17 @@ fn resolve_mcp_command(
/// The two transports run in different places, so they keep different
/// contracts: a stdio script runs on the host outside the sandbox API, while a
/// sandbox script is evaluated by the sandbox's Bash.
#[derive(Clone, Copy)]
#[derive(Clone, Copy, strum::IntoStaticStr)]
enum ScriptInterpreter {
/// Host shell, unchanged for `type = "stdio"` servers.
#[strum(serialize = "sh")]
HostShell,
/// The sandbox's non-login Bash. Resolved through `PATH` rather than
/// spelled `/bin/bash` so local sandboxes keep working on NixOS.
#[strum(serialize = "bash")]
SandboxBash,
}
impl ScriptInterpreter {
fn executable(self) -> &'static str {
match self {
Self::HostShell => "sh",
Self::SandboxBash => "bash",
}
}
}
fn resolve_hook(hook: &HookEntry, index: usize, errors: &mut Vec<ResolveError>) -> HookDefinition {
let variants = [
hook.script.is_some() || hook.command.is_some(),

2
lib/foundation/fabro-dev/src/commands/docs_options_reference.rs
View file

@ -375,7 +375,7 @@ tool_timeout = "90s"
|---|---|---|---|
| `type` | `"stdio"` \| `"http"` \| `"sandbox"` | None | MCP transport type. |
| `command` | array<string> | None | Command and arguments for `stdio` or `sandbox` transports. |
| `script` | string | None | Shell script alternative to `command` for process-launching transports. |
| `script` | string | None | Shell script alternative to `command` for process-launching transports. A `stdio` script runs on the host through `sh -c`; a `sandbox` script is evaluated inside the sandbox by non-login Bash. |
| `url` | string | None | Remote MCP URL for `http` transport. |
| `port` | integer | None | Sandbox port for `sandbox` transport. |
| `env` | table | `{}` | Additional environment variables for process-launching transports. |