scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-17 23:52:34 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Harden run-tool target and workflow resolution

Browse source
This commit is contained in:
Scott Werner 2026-09-03 12:01:33 -04:00
parent c2d6dc4a92
commit b12eb8d84e
10 changed files with 225 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -153,7 +153,7 @@ You can also reuse an exact immutable workflow version without uploading content
}
```
Workflow source and run target are independent. Object-form calls can provide an explicit Git, `none`, or folder target. A workflow agent inherits its parent run's canonical target when `target` is omitted. Standalone MCP derives an attached GitHub checkout only when it can prove that the exact local HEAD is available from the canonical origin; otherwise, push the commit or provide an explicit target. Docker and Daytona agents must send inline files or a stored version ID—never send a sandbox path as a selector, because the run worker cannot use that path to read the sandbox's files.
Workflow source and run target are independent. Object-form calls can provide an explicit Git, `none`, or folder target. A workflow agent inherits its parent run's canonical target when `target` is omitted. Folder targets are available only to standalone MCP and Local workflow agents with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment. Standalone MCP derives an attached GitHub checkout only when it can prove that the exact local HEAD is available from the canonical origin; otherwise, push the commit or provide an explicit target. Docker and Daytona agents must send inline files or a stored version ID—never send a sandbox path as a selector, because the run worker cannot use that path to read the sandbox's files.
Use `goal` for inline goal text or `goal_file` to read the run goal from a file. They are mutually exclusive. Relative `goal_file` paths resolve from the run's `cwd`, or from the MCP server working directory when `cwd` is omitted. Like selectors, `goal_file` requires a native/shared filesystem; Docker and Daytona agents must send `goal` text by value.

2
docs/public/execution/child-runs.mdx
View file

@ -134,7 +134,7 @@ Reuse content already registered with Fabro by supplying its exact immutable ID:
}
```
Workflow content and workspace target are separate choices. If `target` is omitted, a child inherits the parent's full canonical Git, `none`, or folder target. An explicit Git, `none`, or folder target overrides that inheritance while the current run remains the forced parent.
Workflow content and workspace target are separate choices. If `target` is omitted, a child inherits the parent's full canonical Git, `none`, or folder target. An explicit Git, `none`, or folder target overrides that inheritance while the current run remains the forced parent. Folder targets require a Local parent with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment.
`goal_file` is also a shared-filesystem feature. Local agents can read it relative to the operation `cwd`; Docker and Daytona agents must send the resolved `goal` text by value.

8
lib/apps/fabro-cli/src/commands/run/runner.rs
View file

@ -6,7 +6,7 @@ use std::time::Duration;
use anyhow::{Context, Result, anyhow};
use async_trait::async_trait;
use fabro_client::ServerTarget;
use fabro_config::user::active_settings_path;
use fabro_config::user::default_workflows_dir;
use fabro_config::{ServerSettingsBuilder, Storage};
use fabro_interview::{
AnswerSubmission, ControlInterviewer, WORKER_CONTROL_INVALID_CURSOR_REASON,
@ -241,12 +241,8 @@ fn build_fabro_run_tool_services(
if worker_token.trim().is_empty() {
return None;
}
let settings_path = active_settings_path(None);
let user_workflows_root = settings_path
.parent()
.map(|parent| parent.join("workflows"));
let backend = ClientBackend::new(Arc::new(client)).with_run_create_adapter(Arc::new(
ServerRunCreateAdapter::worker(provider, inherited_target, user_workflows_root),
ServerRunCreateAdapter::worker(provider, inherited_target, Some(default_workflows_dir())),
)).with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager));
Some(FabroRunToolServices {
backend: Arc::new(backend),

10
lib/apps/fabro-mcp-server/src/server.rs
View file

@ -4,6 +4,7 @@ use std::time::Duration;
use anyhow::Result;
use fabro_manifest::SuppliedWorkflowVersionPackager;
use fabro_config::user;
use fabro_server::run_tool_create::ServerRunCreateAdapter;
use fabro_tool::fabro_client::ClientBackend;
use fabro_tool::{self as run_tools, FabroToolBackend};
@ -273,14 +274,11 @@ impl FabroMcpServer {
(self.settings.client_factory)()
.await
.map(|client| {
let user_workflows_root = self
.settings
.config_path
.parent()
.map(|parent| parent.join("workflows"));
Arc::new(
ClientBackend::new(Arc::new(client)).with_run_create_adapter(Arc::new(
ServerRunCreateAdapter::standalone(user_workflows_root),
ServerRunCreateAdapter::standalone(Some(
user::default_workflows_dir(),
)).with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager)),
)).with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager)),
) as Arc<dyn FabroToolBackend>
})

10
lib/apps/fabro-server/src/run_intent.rs
View file

@ -7,8 +7,8 @@ use fabro_environment::{EnvironmentId, EnvironmentValidationError};
use fabro_manifest::CollectedWorkflowClosure;
use fabro_types::settings::InterpString;
use fabro_types::{
GitContext, ManifestPath, RunTarget, SandboxProviderKind, TargetValidationError, WorkflowPath,
WorkflowVersion, WorkflowVersionId,
GitContext, ManifestPath, RunId, RunTarget, SandboxProviderKind, TargetValidationError,
WorkflowPath, WorkflowVersion, WorkflowVersionId,
};
use fabro_workflow::git;
use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle};
@ -40,6 +40,12 @@ pub(crate) enum RunIntentAdmissionError {
#[source]
source: fabro_variable::Error,
},
#[error("originating worker run `{run_id}` could not be loaded")]
WorkerRun {
run_id: RunId,
#[source]
source: fabro_store::Error,
},
}
#[derive(Debug, Error)]

57
lib/apps/fabro-server/src/run_tool_create.rs
View file

@ -104,6 +104,11 @@ impl ServerRunCreateAdapter {
cwd: &Path,
) -> Result<ResolvedTarget> {
if let Some(target) = &spec.target {
if matches!(target, RunTarget::Folder { .. }) && !self.has_shared_filesystem() {
bail!(
"folder targets require a shared Local filesystem; Docker and Daytona parents cannot select server-host folders"
);
}
return Ok(ResolvedTarget {
target: target.clone(),
warnings: Vec::new(),
@ -552,6 +557,58 @@ mod tests {
assert!(goal_error.to_string().contains("send goal text by value"));
}
#[tokio::test]
async fn workflow_version_clone_based_workers_reject_explicit_folder_targets() {
let client = no_proxy_client("http://127.0.0.1:9");
let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
let spec = validated_spec(&json!({
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
},
"target": { "kind": "folder", "path": "/srv/server-workspace" }
}));
for provider in [EnvironmentProvider::Docker, EnvironmentProvider::Daytona] {
let adapter = ServerRunCreateAdapter::worker(provider, None, None);
let error = adapter
.prepare(&client, &spec, Path::new("/ignored"))
.await
.expect_err("clone-based workers must not select server-host folders");
assert!(
error
.to_string()
.contains("cannot select server-host folders"),
"unexpected error for {provider:?}: {error:#}"
);
}
}
#[tokio::test]
async fn workflow_version_local_worker_accepts_explicit_folder_target() {
let client = no_proxy_client("http://127.0.0.1:9");
let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
let target = RunTarget::Folder {
path: "/srv/server-workspace".to_string(),
};
let spec = validated_spec(&json!({
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
},
"target": target
}));
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Local, None, None);
let prepared = adapter
.prepare(&client, &spec, Path::new("/ignored"))
.await
.unwrap();
assert_eq!(prepared.target, target);
}
#[tokio::test]
async fn workflow_version_is_registered_before_server_admission_rejection() {
let server = MockServer::start_async().await;

43
lib/apps/fabro-server/src/server/handler/runs.rs
View file

@ -631,6 +631,9 @@ pub(crate) async fn create_run_from_intent(
Ok(validated) => validated,
Err(error) => return run_intent_admission_error(error.into()),
};
if let Err(error) = validate_intent_actor_target(&state, &actor, &target).await {
return run_intent_admission_error(error);
}
let environment_id = match select_intent_environment_id(
&state,
intent
@ -985,6 +988,7 @@ fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response {
match &error {
RunIntentAdmissionError::VersionStore { .. }
| RunIntentAdmissionError::VariableSnapshot { .. }
| RunIntentAdmissionError::WorkerRun { .. }
| RunIntentAdmissionError::Environment(EnvironmentSelectionError::CredentialStore {
..
}) => {
@ -1076,9 +1080,48 @@ fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response {
"failed to load run variables",
"variable_store_error",
),
RunIntentAdmissionError::WorkerRun { .. } => intent_error(
StatusCode::INTERNAL_SERVER_ERROR,
"failed to inspect originating worker run",
"worker_run_store_error",
),
}
}
async fn validate_intent_actor_target(
state: &AppState,
actor: &Principal,
target: &RunTarget,
) -> Result<(), RunIntentAdmissionError> {
let (Principal::Worker { run_id }, RunTarget::Folder { .. }) = (actor, target) else {
return Ok(());
};
let run_store = state
.stores
.runs
.open_run_reader(run_id)
.await
.map_err(|source| RunIntentAdmissionError::WorkerRun {
run_id: *run_id,
source,
})?;
let projection =
run_store
.state()
.await
.map_err(|source| RunIntentAdmissionError::WorkerRun {
run_id: *run_id,
source,
})?;
if !projection.spec.settings.run.environment.provider.is_local() {
return Err(EnvironmentSelectionError::TargetUnsupported {
detail: "folder targets created by a worker require a Local parent environment",
}
.into());
}
Ok(())
}
fn select_intent_environment_id(
state: &AppState,
value: &str,

83
lib/apps/fabro-server/src/server/tests.rs
View file

@ -4494,6 +4494,89 @@ enabled = false
);
}
#[tokio::test]
async fn run_tools_worker_cannot_select_server_folder_from_clone_based_parent() {
let dir = tempfile::tempdir().unwrap();
let missing_target = dir.path().join("missing");
let (state, app) = jwt_auth_app();
let user_token = issue_test_user_jwt();
let parent_run_id = create_run_with_bearer(&app, &user_token).await;
let worker_token = issue_test_run_tools_worker_token(&parent_run_id);
let workflow_version_id = store_workflow_version(&state, MINIMAL_DOT, None).await;
let mut intent = folder_intent(workflow_version_id, missing_target.to_string_lossy());
intent["environment_id"] = json!("local");
intent["parent_id"] = json!(parent_run_id);
let response = app
.oneshot(json_bearer_request(
Method::POST,
"/runs",
&worker_token,
&intent,
))
.await
.unwrap();
let body = response_json!(response, StatusCode::UNPROCESSABLE_ENTITY).await;
assert_eq!(body["errors"][0]["code"], "target_environment_unsupported");
assert_eq!(
body["errors"][0]["detail"],
"folder targets created by a worker require a Local parent environment"
);
assert_eq!(
state
.stores
.run_summaries
.list_identities()
.await
.unwrap()
.len(),
1,
"the rejected child must not be persisted"
);
}
#[tokio::test]
async fn run_tools_worker_can_select_server_folder_from_local_parent() {
let dir = tempfile::tempdir().unwrap();
let (state, app) = jwt_auth_app();
let user_token = issue_test_user_jwt();
let workflow_version_id = store_workflow_version(&state, MINIMAL_DOT, None).await;
let mut parent_intent = folder_intent(workflow_version_id, dir.path().to_string_lossy());
parent_intent["environment_id"] = json!("local");
let response = app
.clone()
.oneshot(json_bearer_request(
Method::POST,
"/runs",
&user_token,
&parent_intent,
))
.await
.unwrap();
let parent = response_json!(response, StatusCode::CREATED).await;
let parent_run_id = parent["id"].as_str().unwrap().parse::<RunId>().unwrap();
let worker_token = issue_test_run_tools_worker_token(&parent_run_id);
let mut child_intent = folder_intent(workflow_version_id, dir.path().to_string_lossy());
child_intent["environment_id"] = json!("local");
child_intent["parent_id"] = json!(parent_run_id);
let response = app
.oneshot(json_bearer_request(
Method::POST,
"/runs",
&worker_token,
&child_intent,
))
.await
.unwrap();
let child = response_json!(response, StatusCode::CREATED).await;
assert_eq!(child["parent_id"], parent_run_id.to_string());
assert_eq!(child["lifecycle"]["status"]["kind"], "submitted");
}
#[tokio::test]
async fn post_runs_run_intent_accepts_none_target_with_ready_daytona_environment() {
let state = TestAppStateBuilder::new()

2
lib/components/fabro-tool/src/create.rs
View file

@ -128,7 +128,7 @@ impl JsonSchema for CreateRunSpecInput {
"description": "Optional parent run id or selector."
},
"target": {
"description": "Canonical run workspace target. Worker calls inherit the parent target when omitted; standalone calls require an attached GitHub checkout whose exact local HEAD is available from the canonical origin.",
"description": "Canonical run workspace target. Worker calls inherit the parent target when omitted; standalone calls require an attached GitHub checkout whose exact local HEAD is available from the canonical origin. Folder targets require a standalone or Local-worker filesystem context.",
"anyOf": [
{ "type": "null" },
{

27
lib/foundation/fabro-config/src/user.rs
View file

@ -23,6 +23,10 @@ pub fn default_storage_dir() -> PathBuf {
Home::from_env().root().join("storage")
}
pub fn default_workflows_dir() -> PathBuf {
Home::from_env().workflows_dir()
}
pub fn default_socket_path() -> PathBuf {
Home::from_env().root().join("fabro.sock")
}
@ -77,8 +81,8 @@ mod tests {
use temp_env::with_var;
use super::{
SETTINGS_CONFIG_FILENAME, active_settings_path_with_lookup, default_settings_path,
default_socket_path, default_storage_dir,
SETTINGS_CONFIG_FILENAME, active_settings_path, active_settings_path_with_lookup,
default_settings_path, default_socket_path, default_storage_dir, default_workflows_dir,
};
#[test]
@ -91,10 +95,29 @@ mod tests {
home.join(".fabro").join(SETTINGS_CONFIG_FILENAME)
);
assert_eq!(default_storage_dir(), home.join(".fabro/storage"));
assert_eq!(default_workflows_dir(), home.join(".fabro/workflows"));
assert_eq!(default_socket_path(), home.join(".fabro/fabro.sock"));
});
}
#[test]
fn workflows_path_uses_fabro_home_when_config_is_elsewhere() {
let dir = tempfile::tempdir().unwrap();
let fabro_home = dir.path().join("fabro-home");
let custom_config = dir.path().join("config/settings.toml");
with_var(EnvVars::FABRO_HOME, Some(fabro_home.as_os_str()), || {
with_var(
EnvVars::FABRO_CONFIG,
Some(custom_config.as_os_str()),
|| {
assert_eq!(active_settings_path(None), custom_config);
assert_eq!(default_workflows_dir(), fabro_home.join("workflows"));
},
);
});
}
#[test]
fn active_settings_path_honors_fabro_config_env() {
let dir = tempfile::tempdir().unwrap();