mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
fix(sandbox): materialize prompt blobs in runtime storage, not the checkout
Remote prompt-value materialization wrote demoted values to
{working_directory}/.fabro/blobs inside the repository checkout, so a
later checkpoint could commit them and leak them into the run pull
request.
Give each sandbox a run-scoped runtime directory outside the source
checkout as part of the Sandbox contract:
- Sandbox::runtime_directory() names the directory; host-local
sandboxes return None because the engine owns a host-side runtime
directory (RunScratch) for those runs.
- Docker creates /fabro/runtime at initialize with umask 077 and
uploads runtime files with mode 0600.
- Daytona creates /home/daytona/fabro/runtime with mode 0700.
- Both remote materialization paths in fabro-workflow share one
materialization-path helper built on the new contract. The paths keep
the runtime/blobs suffix, so durable context still normalizes to
blob://sha256/... references.
- Local materialization now writes owner-private directories and files
on Unix.
Regression coverage: an integration test runs remote-style prompt
demotion against a real git checkout, then a real checkpoint commit,
and asserts the checkout stays clean, the agent-facing file is
readable, and a deleted materialized file is recreated from the
durable blob store. A real-Docker test verifies the runtime directory
and blob file permissions inside a container.
Fixes #798
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kmn5jyrdpyCdvcfvmEDvA
This commit is contained in:
parent
7ae7ca9ead
commit
b4fd7ae00b
10 changed files with 595 additions and 35 deletions
|
|
@ -237,7 +237,7 @@ Captured stage artifacts such as screenshots, videos, reports, and traces still
|
|||
|
||||
For remote sandboxes (Docker, Daytona), execution-time file access happens inside the sandbox filesystem.
|
||||
|
||||
- Blob refs are materialized into `{working_directory}/.fabro/blobs/{blob_hash}.json`
|
||||
- Blob refs are materialized into the sandbox runtime directory, `{runtime_directory}/blobs/{blob_hash}.json`. This directory lives outside the repository checkout, so materialized blobs never show up in `git status` or in checkpoint commits.
|
||||
- Explicit non-blob `file://` refs keep the existing copy-on-demand behavior and are copied into `{working_directory}/.fabro/artifacts/{filename}` when needed
|
||||
|
||||
In both cases, downstream handlers and agents continue to consume ordinary `file://` pointers during execution.
|
||||
|
|
|
|||
|
|
@ -250,7 +250,7 @@ Checkpoints and checkpoint-completed events persist these `blob://` refs, not ho
|
|||
Before Fabro builds a preamble or starts the next stage, it resolves any blob refs into execution-local files so handlers and agents still see normal `file://` references:
|
||||
|
||||
- Local execution materializes blobs under `{run_dir}/runtime/blobs/{blob_hash}.json`
|
||||
- Remote sandboxes materialize blobs under `{working_directory}/.fabro/blobs/{blob_hash}.json`
|
||||
- Remote sandboxes materialize blobs under the sandbox runtime directory, `{runtime_directory}/blobs/{blob_hash}.json`. This directory lives outside the repository checkout, so materialized blobs never appear in `git status` and are never committed by a checkpoint.
|
||||
|
||||
These materialized `file://` paths are runtime-only. They are not written back into durable context snapshots.
|
||||
|
||||
|
|
|
|||
|
|
@ -61,6 +61,7 @@ const DAYTONA_BASH_SESSION_REMEDIATION: &str = "Daytona ran the direct command t
|
|||
|
||||
pub(crate) const WORKING_DIRECTORY: &str = "/home/daytona/workspace";
|
||||
pub(crate) const REPOS_ROOT: &str = "/home/daytona/repos";
|
||||
pub(crate) const RUNTIME_DIRECTORY: &str = "/home/daytona/fabro/runtime";
|
||||
const DEFAULT_SNAPSHOT: &str = "daytona-medium";
|
||||
pub const DEFAULT_DAYTONA_API_URL: &str = "https://app.daytona.io/api";
|
||||
pub(crate) const DAYTONA_DASHBOARD_SANDBOXES_URL: &str =
|
||||
|
|
@ -786,6 +787,41 @@ impl DaytonaSandbox {
|
|||
Self::probe_bash_session(sandbox).await
|
||||
}
|
||||
|
||||
/// Create the run-scoped Fabro runtime directory outside the repository
|
||||
/// checkout, with owner-private permissions on each created level.
|
||||
async fn create_runtime_directory(sandbox: &daytona_sdk::Sandbox) -> crate::Result<()> {
|
||||
let fs_svc = sandbox
|
||||
.fs()
|
||||
.await
|
||||
.map_err(|e| crate::Error::context("Failed to get Daytona fs service", e))?;
|
||||
let runtime_parent = Path::new(RUNTIME_DIRECTORY)
|
||||
.parent()
|
||||
.map(|parent| parent.to_string_lossy().to_string());
|
||||
if let Some(runtime_parent) = runtime_parent {
|
||||
fs_svc
|
||||
.create_folder(&runtime_parent, Some("0700"))
|
||||
.await
|
||||
.map_err(|e| {
|
||||
wrap_fs_error(
|
||||
"Failed to create Daytona runtime parent directory",
|
||||
&runtime_parent,
|
||||
e,
|
||||
)
|
||||
})?;
|
||||
}
|
||||
fs_svc
|
||||
.create_folder(RUNTIME_DIRECTORY, Some("0700"))
|
||||
.await
|
||||
.map_err(|e| {
|
||||
wrap_fs_error(
|
||||
"Failed to create Daytona runtime directory",
|
||||
RUNTIME_DIRECTORY,
|
||||
e,
|
||||
)
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Probe Bash over the direct process-exec transport.
|
||||
async fn probe_bash_exec(sandbox: &daytona_sdk::Sandbox) -> crate::Result<()> {
|
||||
let start = Instant::now();
|
||||
|
|
@ -1470,6 +1506,13 @@ impl Sandbox for DaytonaSandbox {
|
|||
return Err(self.fail_init(init_start, err));
|
||||
}
|
||||
|
||||
if let Err(runtime_error) = Self::create_runtime_directory(&sandbox).await {
|
||||
let err = self
|
||||
.finish_failed_initialization(sandbox, runtime_error)
|
||||
.await;
|
||||
return Err(self.fail_init(init_start, err));
|
||||
}
|
||||
|
||||
let clone_decision = clone_source::decide_clone(
|
||||
self.config.skip_clone,
|
||||
self.clone_origin_url.as_deref(),
|
||||
|
|
@ -1872,6 +1915,10 @@ impl Sandbox for DaytonaSandbox {
|
|||
.map_or(WORKING_DIRECTORY, String::as_str)
|
||||
}
|
||||
|
||||
fn runtime_directory(&self) -> Option<&str> {
|
||||
Some(RUNTIME_DIRECTORY)
|
||||
}
|
||||
|
||||
fn platform(&self) -> &'static str {
|
||||
"linux"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ const DOCKER_BASH_REQUIREMENT: &str = "Docker sandboxes require /bin/bash for ev
|
|||
|
||||
pub(crate) const WORKING_DIRECTORY: &str = "/workspace";
|
||||
pub(crate) const REPOS_ROOT: &str = "/repos";
|
||||
pub(crate) const RUNTIME_DIRECTORY: &str = "/fabro/runtime";
|
||||
const DEFAULT_GIT_CLONE_DEPTH: usize = RunCloneSettings::DEFAULT_DEPTH.unsigned_abs() as usize;
|
||||
const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(5);
|
||||
#[cfg(test)]
|
||||
|
|
@ -717,6 +718,28 @@ impl DockerSandbox {
|
|||
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
|
||||
|
|
@ -1144,14 +1167,16 @@ impl DockerSandbox {
|
|||
.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(
|
||||
&format!("mkdir -p {}", shell_quote(&parent_dir)),
|
||||
10_000,
|
||||
Some("/"),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.docker_exec_shell(&mkdir_cmd, 10_000, Some("/"), None, None)
|
||||
.await?;
|
||||
if !result.is_success() {
|
||||
return Err(crate::Error::message(format!(
|
||||
|
|
@ -1160,7 +1185,8 @@ impl DockerSandbox {
|
|||
)));
|
||||
}
|
||||
|
||||
let tar_bytes = build_single_file_tar(&file_name, bytes)?;
|
||||
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(),
|
||||
|
|
@ -1660,7 +1686,7 @@ 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>> {
|
||||
fn build_single_file_tar(file_name: &str, bytes: &[u8], mode: u32) -> crate::Result<Vec<u8>> {
|
||||
let mut tar_builder = tar::Builder::new(Vec::new());
|
||||
let mut header = tar::Header::new_gnu();
|
||||
header
|
||||
|
|
@ -1670,7 +1696,7 @@ fn build_single_file_tar(file_name: &str, bytes: &[u8]) -> crate::Result<Vec<u8>
|
|||
u64::try_from(bytes.len())
|
||||
.map_err(|_| crate::Error::message("file is too large for tar header"))?,
|
||||
);
|
||||
header.set_mode(0o644);
|
||||
header.set_mode(mode);
|
||||
header.set_cksum();
|
||||
tar_builder
|
||||
.append(&header, bytes)
|
||||
|
|
@ -1788,6 +1814,10 @@ impl Sandbox for DockerSandbox {
|
|||
.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(),
|
||||
|
|
@ -2313,6 +2343,10 @@ impl Sandbox for DockerSandbox {
|
|||
.map_or(WORKING_DIRECTORY, String::as_str)
|
||||
}
|
||||
|
||||
fn runtime_directory(&self) -> Option<&str> {
|
||||
Some(RUNTIME_DIRECTORY)
|
||||
}
|
||||
|
||||
async fn ssh_access_command(&self) -> crate::Result<Option<String>> {
|
||||
Ok(Some(docker_access_command(
|
||||
self.container_id()?,
|
||||
|
|
@ -3005,16 +3039,26 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn single_file_tar_contains_named_file() {
|
||||
let bytes = build_single_file_tar("nested.txt", b"hello").unwrap();
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -1441,6 +1441,24 @@ pub trait Sandbox: Send + Sync {
|
|||
}
|
||||
async fn cleanup(&self) -> crate::Result<()>;
|
||||
fn working_directory(&self) -> &str;
|
||||
/// Run-scoped directory for Fabro-owned runtime files inside the sandbox,
|
||||
/// or `None` when the sandbox has no such directory.
|
||||
///
|
||||
/// The directory sits outside every repository checkout, so runtime files
|
||||
/// Fabro materializes beneath it — for example oversized prompt values
|
||||
/// projected out of the durable blob store — never appear in `git status`
|
||||
/// and can never be committed by a checkpoint. Its contents are
|
||||
/// disposable: everything beneath it can be recreated from durable
|
||||
/// storage on demand.
|
||||
///
|
||||
/// Providers that provision an isolated per-run environment (Docker,
|
||||
/// Daytona) create the directory during initialization with private
|
||||
/// permissions and return its path. Sandboxes that execute directly on
|
||||
/// the worker host return `None`; the workflow engine owns a host-side
|
||||
/// runtime directory for those runs.
|
||||
fn runtime_directory(&self) -> Option<&str> {
|
||||
None
|
||||
}
|
||||
fn platform(&self) -> &str;
|
||||
fn os_version(&self) -> String;
|
||||
/// Return a human-readable identifier for the sandbox (e.g. container ID,
|
||||
|
|
|
|||
|
|
@ -395,3 +395,78 @@ async fn docker_glob_matches_patterns_containing_a_path_separator() {
|
|||
"`**/SKILL.md` should match files nested several levels deep, got: {recursive:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// The Fabro runtime directory is where prompt blobs materialize, so it must
|
||||
// exist after initialization, sit outside the repository checkout, and stay
|
||||
// owner-private along with the files written beneath it (issue #798).
|
||||
#[tokio::test]
|
||||
#[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() {
|
||||
return;
|
||||
}
|
||||
|
||||
let sandbox = DockerSandbox::new(
|
||||
DockerSandboxOptions {
|
||||
image: image.to_string(),
|
||||
auto_pull: false,
|
||||
skip_clone: true,
|
||||
..DockerSandboxOptions::default()
|
||||
},
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.expect("docker sandbox should construct");
|
||||
sandbox
|
||||
.initialize()
|
||||
.await
|
||||
.expect("docker sandbox should initialize");
|
||||
|
||||
let runtime_directory = sandbox
|
||||
.runtime_directory()
|
||||
.expect("docker sandbox should expose a runtime directory")
|
||||
.to_string();
|
||||
assert!(
|
||||
!runtime_directory.starts_with(sandbox.working_directory()),
|
||||
"runtime directory {runtime_directory} must sit outside the workspace"
|
||||
);
|
||||
|
||||
let blob_path = format!("{runtime_directory}/blobs/test-blob.json");
|
||||
sandbox
|
||||
.write_file(&blob_path, "{}")
|
||||
.await
|
||||
.expect("runtime blob write should succeed");
|
||||
|
||||
let modes = sandbox
|
||||
.exec_command(
|
||||
&format!("stat -c '%a' {runtime_directory} {blob_path}"),
|
||||
10_000,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("stat should run");
|
||||
let readback = sandbox.read_file_text(&blob_path).await;
|
||||
|
||||
sandbox
|
||||
.cleanup()
|
||||
.await
|
||||
.expect("docker cleanup should succeed");
|
||||
|
||||
assert!(modes.is_success(), "stat failed: {}", modes.stderr);
|
||||
let modes: Vec<&str> = modes.stdout.split_whitespace().collect();
|
||||
assert_eq!(
|
||||
modes,
|
||||
["700", "600"],
|
||||
"runtime directory and blob file should be owner-private"
|
||||
);
|
||||
assert_eq!(readback.expect("runtime blob should be readable"), "{}");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ use fabro_types::{
|
|||
use futures::future::BoxFuture;
|
||||
use serde_json::Value;
|
||||
use tokio::fs;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
|
||||
use crate::context::{self, Context};
|
||||
use crate::error::{Error, Result};
|
||||
|
|
@ -295,22 +296,40 @@ async fn materialize_value_bytes(
|
|||
return Ok(path.display().to_string());
|
||||
}
|
||||
|
||||
let remote_path = format!("{}/.fabro/blobs/{blob_hash}.json", env.working_directory());
|
||||
let remote_path = remote_materialized_blob_path(env, &blob_hash)?;
|
||||
if !env
|
||||
.file_exists(&remote_path)
|
||||
.await
|
||||
.map_err(|e| Error::engine_with_source("failed to check blob existence", e))?
|
||||
{
|
||||
persist_blob(bytes, run_store).await?;
|
||||
let content = std::str::from_utf8(bytes)
|
||||
.map_err(|e| Error::engine_with_source("artifact blob was not valid UTF-8 JSON", e))?;
|
||||
env.write_file(&remote_path, content).await.map_err(|e| {
|
||||
Error::engine_with_source("failed to write artifact blob to sandbox", e)
|
||||
})?;
|
||||
write_remote_blob_file(env, &remote_path, bytes).await?;
|
||||
}
|
||||
Ok(remote_path)
|
||||
}
|
||||
|
||||
/// The sandbox file that materializes one blob for agent reads.
|
||||
///
|
||||
/// The file lives beneath the sandbox's run-scoped runtime directory, never
|
||||
/// the repository checkout, so materialization cannot dirty `git status` and
|
||||
/// a later checkpoint can never commit it. The `runtime/blobs` suffix keeps
|
||||
/// the path recognizable as a managed blob reference, so durable storage
|
||||
/// still records `blob://sha256/...` instead of this execution-local path.
|
||||
fn remote_materialized_blob_path(env: &dyn Sandbox, blob_hash: &BlobHash) -> Result<String> {
|
||||
let runtime_directory = env.runtime_directory().ok_or_else(|| {
|
||||
Error::engine("sandbox exposes no runtime directory for blob materialization")
|
||||
})?;
|
||||
Ok(format!("{runtime_directory}/blobs/{blob_hash}.json"))
|
||||
}
|
||||
|
||||
async fn write_remote_blob_file(env: &dyn Sandbox, path: &str, bytes: &[u8]) -> Result<()> {
|
||||
let content = std::str::from_utf8(bytes)
|
||||
.map_err(|e| Error::engine_with_source("artifact blob was not valid UTF-8 JSON", e))?;
|
||||
env.write_file(path, content)
|
||||
.await
|
||||
.map_err(|e| Error::engine_with_source("failed to write artifact blob to sandbox", e))
|
||||
}
|
||||
|
||||
async fn persist_blob(bytes: &[u8], run_store: &RunStoreHandle) -> Result<()> {
|
||||
run_store
|
||||
.write_blob(bytes)
|
||||
|
|
@ -715,33 +734,43 @@ async fn materialize_blob_ref(
|
|||
return Ok(format!("{ARTIFACT_POINTER_PREFIX}{}", path.display()));
|
||||
}
|
||||
|
||||
let remote_path = format!("{}/.fabro/blobs/{blob_hash}.json", env.working_directory());
|
||||
let remote_path = remote_materialized_blob_path(env, blob_hash)?;
|
||||
if !env
|
||||
.file_exists(&remote_path)
|
||||
.await
|
||||
.map_err(|e| Error::engine_with_source("failed to check blob existence", e))?
|
||||
{
|
||||
let bytes = read_required_blob(blob_hash, run_store).await?;
|
||||
let content = String::from_utf8(bytes.to_vec())
|
||||
.map_err(|e| Error::engine_with_source("artifact blob was not valid UTF-8 JSON", e))?;
|
||||
env.write_file(&remote_path, &content).await.map_err(|e| {
|
||||
Error::engine_with_source("failed to write artifact blob to sandbox", e)
|
||||
})?;
|
||||
write_remote_blob_file(env, &remote_path, &bytes).await?;
|
||||
}
|
||||
|
||||
Ok(format!("{ARTIFACT_POINTER_PREFIX}{remote_path}"))
|
||||
}
|
||||
|
||||
/// Write a materialized blob file, keeping created directories and the file
|
||||
/// itself owner-private where the platform supports modes.
|
||||
async fn write_local_blob_file(path: &Path, bytes: &[u8]) -> Result<()> {
|
||||
if let Some(parent) = path.parent() {
|
||||
fs::create_dir_all(parent).await.map_err(|err| {
|
||||
let mut builder = fs::DirBuilder::new();
|
||||
builder.recursive(true);
|
||||
#[cfg(unix)]
|
||||
builder.mode(0o700);
|
||||
builder.create(parent).await.map_err(|err| {
|
||||
Error::Io(format!(
|
||||
"creating artifact blob directory {}: {err}",
|
||||
parent.display()
|
||||
))
|
||||
})?;
|
||||
}
|
||||
fs::write(path, bytes)
|
||||
let mut options = fs::OpenOptions::new();
|
||||
options.write(true).create(true).truncate(true);
|
||||
#[cfg(unix)]
|
||||
options.mode(0o600);
|
||||
let mut file = options
|
||||
.open(path)
|
||||
.await
|
||||
.map_err(|err| Error::Io(format!("writing artifact blob {}: {err}", path.display())))?;
|
||||
file.write_all(bytes)
|
||||
.await
|
||||
.map_err(|err| Error::Io(format!("writing artifact blob {}: {err}", path.display())))
|
||||
}
|
||||
|
|
@ -1269,6 +1298,7 @@ mod tests {
|
|||
accessible: bool,
|
||||
written: Mutex<Vec<(String, String)>>,
|
||||
working_dir: String,
|
||||
runtime_dir: Option<String>,
|
||||
exists_calls: Mutex<usize>,
|
||||
}
|
||||
|
||||
|
|
@ -1278,9 +1308,15 @@ mod tests {
|
|||
accessible,
|
||||
written: Mutex::new(Vec::new()),
|
||||
working_dir: working_dir.to_string(),
|
||||
runtime_dir: None,
|
||||
exists_calls: Mutex::new(0),
|
||||
}
|
||||
}
|
||||
|
||||
fn with_runtime_directory(mut self, runtime_dir: &str) -> Self {
|
||||
self.runtime_dir = Some(runtime_dir.to_string());
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
|
|
@ -1370,6 +1406,10 @@ mod tests {
|
|||
&self.working_dir
|
||||
}
|
||||
|
||||
fn runtime_directory(&self) -> Option<&str> {
|
||||
self.runtime_dir.as_deref()
|
||||
}
|
||||
|
||||
fn platform(&self) -> &str {
|
||||
"linux"
|
||||
}
|
||||
|
|
@ -1499,6 +1539,100 @@ mod tests {
|
|||
assert!(details["preview"].as_str().unwrap().starts_with("ooo"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn demote_materializes_remote_values_under_sandbox_runtime_directory() {
|
||||
let run_store: RunStoreHandle = make_run_store("prompt-demote-remote").await.into();
|
||||
let run_dir = tempfile::tempdir().unwrap();
|
||||
let env = TestSyncEnv::new(false, "/workspace").with_runtime_directory("/fabro/runtime");
|
||||
|
||||
let oversized = serde_json::json!("x".repeat(PROMPT_INLINE_VALUE_MAX + 1));
|
||||
let expected_bytes = serde_json::to_vec(&oversized).unwrap();
|
||||
let expected_path = format!(
|
||||
"/fabro/runtime/blobs/{}.json",
|
||||
BlobHash::new(&expected_bytes)
|
||||
);
|
||||
let mut values = HashMap::from([("dataset".to_string(), oversized)]);
|
||||
|
||||
demote_large_values_for_prompt(
|
||||
&mut values,
|
||||
&mut HashMap::new(),
|
||||
&run_store,
|
||||
&env,
|
||||
run_dir.path(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let details = prompt_large_value(&values["dataset"])
|
||||
.expect("oversized remote context value should demote");
|
||||
assert_eq!(details.path, expected_path);
|
||||
let written = env.written.lock().unwrap();
|
||||
assert_eq!(written.len(), 1);
|
||||
assert_eq!(written[0].0, expected_path);
|
||||
assert_eq!(written[0].1.as_bytes(), expected_bytes);
|
||||
assert!(
|
||||
!written[0].0.starts_with("/workspace"),
|
||||
"materialization must stay outside the repository checkout"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn demote_keeps_value_inline_when_sandbox_has_no_runtime_directory() {
|
||||
let run_store: RunStoreHandle = make_run_store("prompt-demote-no-runtime").await.into();
|
||||
let run_dir = tempfile::tempdir().unwrap();
|
||||
let env = TestSyncEnv::new(false, "/workspace");
|
||||
|
||||
let oversized = serde_json::json!("x".repeat(PROMPT_INLINE_VALUE_MAX + 1));
|
||||
let mut values = HashMap::from([("dataset".to_string(), oversized.clone())]);
|
||||
|
||||
demote_large_values_for_prompt(
|
||||
&mut values,
|
||||
&mut HashMap::new(),
|
||||
&run_store,
|
||||
&env,
|
||||
run_dir.path(),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(values["dataset"], oversized);
|
||||
assert!(env.written.lock().unwrap().is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_context_materializes_remote_blob_refs_under_runtime_directory() {
|
||||
let run_store = make_run_store("remote-blob-ref-resolution").await;
|
||||
let report = serde_json::json!({"kind": "report"});
|
||||
let report_bytes = serde_json::to_vec(&report).unwrap();
|
||||
let blob_hash = run_store.write_blob(&report_bytes).await.unwrap();
|
||||
let context = Context::new();
|
||||
context.set("report", fabro_types::format_blob_ref(&blob_hash).into());
|
||||
let env = TestSyncEnv::new(false, "/workspace").with_runtime_directory("/fabro/runtime");
|
||||
let run_dir = tempfile::tempdir().unwrap();
|
||||
|
||||
let resolved =
|
||||
resolved_context_snapshot(&context, &run_store.clone().into(), &env, run_dir.path())
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let expected_path = format!("/fabro/runtime/blobs/{blob_hash}.json");
|
||||
assert_eq!(
|
||||
resolved["report"],
|
||||
serde_json::json!(format!("file://{expected_path}"))
|
||||
);
|
||||
let written = env.written.lock().unwrap();
|
||||
assert_eq!(written.len(), 1);
|
||||
assert_eq!(written[0].0, expected_path);
|
||||
assert_eq!(written[0].1.as_bytes(), report_bytes);
|
||||
|
||||
// Durable normalization keeps the blob reference, not the
|
||||
// execution-local runtime path.
|
||||
let mut durable = resolved;
|
||||
normalize_durable_updates(&mut durable);
|
||||
assert_eq!(
|
||||
durable["report"],
|
||||
serde_json::json!(fabro_types::format_blob_ref(&blob_hash))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn demote_skips_keys_the_preamble_never_renders() {
|
||||
let run_store: RunStoreHandle = make_run_store("prompt-demote-hidden").await.into();
|
||||
|
|
|
|||
|
|
@ -788,7 +788,7 @@ mod tests {
|
|||
"security_findings",
|
||||
large_prompt_value(
|
||||
1_843_279,
|
||||
"/workspace/.fabro/blobs/findings.json",
|
||||
"/fabro/runtime/blobs/findings.json",
|
||||
"{\"findings\":[\n{\"severity\":\"high\"}",
|
||||
),
|
||||
);
|
||||
|
|
@ -798,7 +798,7 @@ mod tests {
|
|||
keys::COMMAND_OUTPUT.to_string(),
|
||||
large_prompt_value(
|
||||
12 * 1024,
|
||||
"/workspace/.fabro/blobs/output.json",
|
||||
"/fabro/runtime/blobs/output.json",
|
||||
"first result\nsecond result",
|
||||
),
|
||||
);
|
||||
|
|
@ -819,12 +819,12 @@ mod tests {
|
|||
"\n## Completed stages\n",
|
||||
"- **scan**: succeeded\n",
|
||||
" - Script: `scan --json`\n",
|
||||
" - Output (12.0 KB; full value: `/workspace/.fabro/blobs/output.json`)\n",
|
||||
" - Output (12.0 KB; full value: `/fabro/runtime/blobs/output.json`)\n",
|
||||
" Preview: first result\n",
|
||||
" second result…\n",
|
||||
"\n## Context\n",
|
||||
"- security_findings (1.8 MB; full value: ",
|
||||
"`/workspace/.fabro/blobs/findings.json`)\n",
|
||||
"`/fabro/runtime/blobs/findings.json`)\n",
|
||||
" Preview: {\"findings\":[\n",
|
||||
" {\"severity\":\"high\"}…\n",
|
||||
)
|
||||
|
|
@ -1703,7 +1703,7 @@ mod tests {
|
|||
"security_findings",
|
||||
large_prompt_value(
|
||||
1_843_279,
|
||||
"/workspace/.fabro/blobs/findings.json",
|
||||
"/fabro/runtime/blobs/findings.json",
|
||||
"{\"findings\": [\n{\"message\": \"a | b\"}]}",
|
||||
),
|
||||
);
|
||||
|
|
@ -1718,7 +1718,7 @@ mod tests {
|
|||
|
||||
assert!(preamble.contains(concat!(
|
||||
"| security_findings | 1.8 MB; full value: ",
|
||||
"`/workspace/.fabro/blobs/findings.json`; Preview: ",
|
||||
"`/fabro/runtime/blobs/findings.json`; Preview: ",
|
||||
"{\"findings\": [ {\"message\": \"a \\| b\"}]}… |",
|
||||
)));
|
||||
assert!(!preamble.contains("fabroLargeValue"));
|
||||
|
|
|
|||
|
|
@ -298,3 +298,235 @@ async fn git_checkpoint_skips_start_node() {
|
|||
assert!(!checkpoint_node_ids.contains(&"start"));
|
||||
assert!(checkpoint_node_ids.contains(&"work"));
|
||||
}
|
||||
|
||||
/// Sandbox double for remote-style runs: commands and files operate on a real
|
||||
/// local checkout, but the workflow engine's run directory is reported as
|
||||
/// inaccessible (as it is for Docker/Daytona) and the sandbox exposes a
|
||||
/// runtime directory outside the checkout.
|
||||
struct RemoteRuntimeSandbox {
|
||||
inner: fabro_agent::LocalSandbox,
|
||||
hidden_path: String,
|
||||
runtime_directory: String,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl Sandbox for RemoteRuntimeSandbox {
|
||||
async fn read_file_bytes(&self, path: &str) -> fabro_sandbox::Result<Vec<u8>> {
|
||||
self.inner.read_file_bytes(path).await
|
||||
}
|
||||
|
||||
async fn write_file(&self, path: &str, content: &str) -> fabro_sandbox::Result<()> {
|
||||
self.inner.write_file(path, content).await
|
||||
}
|
||||
|
||||
async fn delete_file(&self, path: &str) -> fabro_sandbox::Result<()> {
|
||||
self.inner.delete_file(path).await
|
||||
}
|
||||
|
||||
async fn file_exists(&self, path: &str) -> fabro_sandbox::Result<bool> {
|
||||
if path == self.hidden_path {
|
||||
return Ok(false);
|
||||
}
|
||||
self.inner.file_exists(path).await
|
||||
}
|
||||
|
||||
async fn list_directory(
|
||||
&self,
|
||||
path: &str,
|
||||
depth: Option<usize>,
|
||||
) -> fabro_sandbox::Result<Vec<fabro_agent::DirEntry>> {
|
||||
self.inner.list_directory(path, depth).await
|
||||
}
|
||||
|
||||
async fn exec_command(
|
||||
&self,
|
||||
command: &str,
|
||||
timeout_ms: u64,
|
||||
working_dir: Option<&str>,
|
||||
env_vars: Option<&HashMap<String, String>>,
|
||||
cancel_token: Option<CancellationToken>,
|
||||
) -> fabro_sandbox::Result<fabro_agent::ExecResult> {
|
||||
self.inner
|
||||
.exec_command(command, timeout_ms, working_dir, env_vars, cancel_token)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn grep(
|
||||
&self,
|
||||
pattern: &str,
|
||||
path: &str,
|
||||
options: &fabro_sandbox::GrepOptions,
|
||||
) -> fabro_sandbox::Result<Vec<String>> {
|
||||
self.inner.grep(pattern, path, options).await
|
||||
}
|
||||
|
||||
async fn download_file_to_local(
|
||||
&self,
|
||||
remote_path: &str,
|
||||
local_path: &Path,
|
||||
) -> fabro_sandbox::Result<()> {
|
||||
self.inner
|
||||
.download_file_to_local(remote_path, local_path)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn upload_file_from_local(
|
||||
&self,
|
||||
local_path: &Path,
|
||||
remote_path: &str,
|
||||
) -> fabro_sandbox::Result<()> {
|
||||
self.inner
|
||||
.upload_file_from_local(local_path, remote_path)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn initialize(&self) -> fabro_sandbox::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn cleanup(&self) -> fabro_sandbox::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn working_directory(&self) -> &str {
|
||||
self.inner.working_directory()
|
||||
}
|
||||
|
||||
fn runtime_directory(&self) -> Option<&str> {
|
||||
Some(&self.runtime_directory)
|
||||
}
|
||||
|
||||
fn platform(&self) -> &str {
|
||||
self.inner.platform()
|
||||
}
|
||||
|
||||
fn os_version(&self) -> String {
|
||||
self.inner.os_version()
|
||||
}
|
||||
}
|
||||
|
||||
fn git_status_porcelain(repo_dir: &Path) -> String {
|
||||
let output = Command::new("git")
|
||||
.args(["status", "--porcelain"])
|
||||
.current_dir(repo_dir)
|
||||
.output()
|
||||
.expect("git status --porcelain should run");
|
||||
assert_success(&output, "git status --porcelain");
|
||||
String::from_utf8(output.stdout).expect("git status output should be UTF-8")
|
||||
}
|
||||
|
||||
fn git_committed_files(repo_dir: &Path, sha: &str) -> String {
|
||||
let output = Command::new("git")
|
||||
.args(["show", "--name-only", "--format=", sha])
|
||||
.current_dir(repo_dir)
|
||||
.output()
|
||||
.expect("git show --name-only should run");
|
||||
assert_success(&output, "git show --name-only");
|
||||
String::from_utf8(output.stdout).expect("git show output should be UTF-8")
|
||||
}
|
||||
|
||||
/// Remote-style prompt demotion must materialize blobs in the sandbox runtime
|
||||
/// directory, outside the checkout, so a real checkpoint commit can never pick
|
||||
/// them up, and re-resolution must recreate a deleted materialized file from
|
||||
/// the durable blob store. Regression test for issue #798.
|
||||
#[tokio::test]
|
||||
async fn remote_prompt_demotion_stays_outside_checkout_and_survives_checkpoint() {
|
||||
use std::time::Duration;
|
||||
|
||||
use fabro_store::test_support as store_test_support;
|
||||
use fabro_types::settings::run::RunCheckpointSettings;
|
||||
use fabro_workflow::context::Context;
|
||||
use fabro_workflow::git::GitAuthor;
|
||||
use fabro_workflow::runtime_store::RunStoreHandle;
|
||||
use fabro_workflow::{artifact, sandbox_git};
|
||||
use object_store::memory::InMemory;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let repo_dir = dir.path().join("repo");
|
||||
init_repo(&repo_dir);
|
||||
let runtime_dir = dir.path().join("fabro").join("runtime");
|
||||
let run_dir = dir.path().join("run");
|
||||
std::fs::create_dir_all(&run_dir).unwrap();
|
||||
|
||||
let sandbox = RemoteRuntimeSandbox {
|
||||
inner: fabro_agent::LocalSandbox::new(repo_dir.clone()),
|
||||
hidden_path: run_dir.to_string_lossy().to_string(),
|
||||
runtime_directory: runtime_dir.to_string_lossy().to_string(),
|
||||
};
|
||||
|
||||
let store = store_test_support::test_database(
|
||||
Arc::new(InMemory::new()),
|
||||
"runs/",
|
||||
Duration::from_millis(1),
|
||||
None,
|
||||
);
|
||||
let run_store: RunStoreHandle = store.create_run(&fixtures::RUN_2).await.unwrap().into();
|
||||
|
||||
let oversized = serde_json::json!("x".repeat(64 * 1024));
|
||||
let oversized_bytes = serde_json::to_vec(&oversized).unwrap();
|
||||
let mut values = HashMap::from([("dataset".to_string(), oversized.clone())]);
|
||||
artifact::demote_large_values_for_prompt(
|
||||
&mut values,
|
||||
&mut HashMap::new(),
|
||||
&run_store,
|
||||
&sandbox,
|
||||
&run_dir,
|
||||
)
|
||||
.await;
|
||||
|
||||
let marker = values["dataset"]
|
||||
.get("fabroLargeValue")
|
||||
.expect("oversized value should demote to a marker");
|
||||
let blob_path = marker["path"].as_str().unwrap().to_string();
|
||||
assert!(
|
||||
blob_path.starts_with(&runtime_dir.to_string_lossy().to_string()),
|
||||
"materialized blob {blob_path} should live under the sandbox runtime directory"
|
||||
);
|
||||
assert!(
|
||||
!blob_path.starts_with(&repo_dir.to_string_lossy().to_string()),
|
||||
"materialized blob {blob_path} must not live inside the checkout"
|
||||
);
|
||||
|
||||
// The agent-facing path is readable through the sandbox.
|
||||
let contents = sandbox.read_file_bytes(&blob_path).await.unwrap();
|
||||
assert_eq!(contents, oversized_bytes);
|
||||
|
||||
// Materialization leaves the checkout clean, and a real checkpoint commit
|
||||
// stages no runtime blob file.
|
||||
assert_eq!(git_status_porcelain(&repo_dir), "");
|
||||
let sha = sandbox_git::git_checkpoint(
|
||||
&sandbox,
|
||||
&fixtures::RUN_2.to_string(),
|
||||
"work",
|
||||
"succeeded",
|
||||
1,
|
||||
None,
|
||||
&RunCheckpointSettings::default(),
|
||||
&GitAuthor::default(),
|
||||
)
|
||||
.await
|
||||
.expect("checkpoint commit should succeed");
|
||||
assert_eq!(git_committed_files(&repo_dir, &sha).trim(), "");
|
||||
assert_eq!(git_status_porcelain(&repo_dir), "");
|
||||
|
||||
// Removing the materialized file and resolving the value again recreates
|
||||
// it from the durable blob store.
|
||||
std::fs::remove_file(&blob_path).unwrap();
|
||||
let blob_hash = fabro_types::BlobHash::new(&oversized_bytes);
|
||||
let context = Context::new();
|
||||
context.set(
|
||||
"report",
|
||||
serde_json::json!(fabro_types::format_blob_ref(&blob_hash)),
|
||||
);
|
||||
let resolved = artifact::resolved_context_snapshot(&context, &run_store, &sandbox, &run_dir)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
resolved["report"],
|
||||
serde_json::json!(format!("file://{blob_path}"))
|
||||
);
|
||||
assert_eq!(
|
||||
sandbox.read_file_bytes(&blob_path).await.unwrap(),
|
||||
oversized_bytes
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10136,6 +10136,10 @@ impl fabro_agent::Sandbox for RemoteMockEnv {
|
|||
Ok(self.existing_paths.lock().unwrap().contains(path))
|
||||
}
|
||||
|
||||
fn runtime_directory(&self) -> Option<&str> {
|
||||
Some("/fabro/runtime")
|
||||
}
|
||||
|
||||
async fn list_directory(
|
||||
&self,
|
||||
_path: &str,
|
||||
|
|
@ -10441,8 +10445,14 @@ async fn downstream_remote_execution_resolves_response_blob_refs_as_text() {
|
|||
assert!(
|
||||
written
|
||||
.iter()
|
||||
.all(|(path, _)| path.contains("/.fabro/blobs/")),
|
||||
"nothing is written outside the sandbox blob directory"
|
||||
.all(|(path, _)| path.starts_with("/fabro/runtime/blobs/")),
|
||||
"nothing is written outside the sandbox runtime blob directory"
|
||||
);
|
||||
assert!(
|
||||
written
|
||||
.iter()
|
||||
.all(|(path, _)| !path.starts_with("/sandbox")),
|
||||
"nothing is written inside the repository checkout"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue