fabro(01KWFBQKBAK5S9EWRSF0TMWM2V): implement (succeeded)

Fabro-Run: 01KWFBQKBAK5S9EWRSF0TMWM2V
Fabro-Completed: 5

⚒️ Generated with [Fabro](https://fabro.sh)
This commit is contained in:
Fabro 2026-07-01 18:23:14 +00:00
parent ca013881f8
commit 1ce366adba
7 changed files with 813 additions and 122 deletions

1
Cargo.lock generated
View file

@ -3294,6 +3294,7 @@ dependencies = [
"serde",
"serde_json",
"sha2 0.10.9",
"shlex",
"tempfile",
"thiserror 2.0.18",
"tokio",

View file

@ -345,13 +345,13 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu
// against the CLI process env — the mirror of the `fabro run` worker
// boundary in `fabro_workflow::operations::start::runtime_mcp_server`.
// Both consumers read the same source-form settings; missing env is a hard
// error and reserved secrets/inputs tokens surface loudly rather than
// leaking.
// error. `fabro exec` has no server vault, so secrets/inputs tokens surface
// loudly rather than leaking.
let mcp_servers = mcp_servers
.into_iter()
.map(|settings| {
settings
.resolve_transport_env(process_env_var)
.resolve_transport_env(process_env_var, |_| None)
.with_context(|| format!("failed to resolve MCP server {:?}", settings.name))
})
.collect::<AnyResult<Vec<_>>>()?;

View file

@ -5,6 +5,7 @@
use std::path::{Path, PathBuf};
use fabro_types::settings::ResolveError;
#[cfg(feature = "daytona")]
use fabro_types::settings::run::DockerfileSource as ResolvedDockerfileSource;
use fabro_types::settings::run::{EnvironmentNetworkMode, RunEnvironmentSettings};
@ -70,8 +71,31 @@ pub fn docker_config_from_environment(
settings: &RunEnvironmentSettings,
skip_clone: bool,
) -> DockerSandboxOptions {
let mut env_vars = settings
.resolve_env(process_env_var)
let env = settings
.resolve_env(process_env_var, |_| None)
.unwrap_or_else(|_| source_env(settings));
docker_config_from_environment_env(settings, skip_clone, env)
}
#[cfg(feature = "docker")]
pub fn docker_config_from_environment_with_secrets(
settings: &RunEnvironmentSettings,
skip_clone: bool,
secrets_lookup: impl FnMut(&str) -> Option<String>,
) -> Result<DockerSandboxOptions, ResolveError> {
let env = settings.resolve_env(process_env_var, secrets_lookup)?;
Ok(docker_config_from_environment_env(
settings, skip_clone, env,
))
}
#[cfg(feature = "docker")]
fn docker_config_from_environment_env(
settings: &RunEnvironmentSettings,
skip_clone: bool,
env: std::collections::HashMap<String, String>,
) -> DockerSandboxOptions {
let mut env_vars = env
.into_iter()
.map(|(key, value)| format!("{key}={value}"))
.collect::<Vec<_>>();
@ -104,6 +128,23 @@ pub fn docker_config_from_environment(
}
}
#[cfg(feature = "docker")]
fn source_env(settings: &RunEnvironmentSettings) -> std::collections::HashMap<String, String> {
settings
.env
.iter()
.map(|(key, value)| {
#[expect(
clippy::disallowed_methods,
reason = "Docker manifest/preflight fallback preserves unresolved run environment \
source when no secret lookup is available"
)]
let source = value.as_source();
(key.clone(), source)
})
.collect()
}
pub fn local_working_directory_from_environment(
settings: &RunEnvironmentSettings,
source_directory: Option<&Path>,

View file

@ -16,7 +16,7 @@ use serde::ser::SerializeStruct;
use serde::{Deserialize, Serialize};
use super::duration::Duration;
use super::interp::{InterpString, Namespace, ResolveError};
use super::interp::{InterpString, Namespace, ResolveCtx, ResolveError};
use super::model_ref::ModelRef;
use super::size::Size;
@ -733,35 +733,37 @@ impl Default for RunPrepareSettings {
}
impl RunPrepareSettings {
/// Resolve `{{ env.* }}` tokens in every prepare step's runnable part and
/// per-step `env` values against `env_lookup`, returning a copy with the
/// tokens replaced and every other field preserved. A `script` step's
/// snippet resolves in place; a `command` step's argv resolves per element
/// (each element is shell-quoted later, in
/// Resolve `{{ env.* }}` and `{{ secrets.* }}` tokens in every prepare
/// step's runnable part and per-step `env` values against the supplied
/// lookups, returning a copy with the tokens replaced and every other field
/// preserved. A `script` step's snippet resolves in place; a `command`
/// step's argv resolves per element (each element is shell-quoted later, in
/// [`PreparedStep::to_shell_command`], so quoting applies to the resolved
/// value rather than the source token).
///
/// This is the late, use-time half of prepare-step interpolation, the
/// counterpart to the server-side `{{ vars.* }}` substitution in
/// [`RunNamespace::substitute_variables`]: `{{ vars.* }}` are substituted
/// earlier, server-side, while `{{ env.* }}` resolve here — in whichever
/// process actually runs the steps (the run worker for `fabro run`).
/// earlier, server-side, while `{{ env.* }}` and `{{ secrets.* }}` resolve
/// here — in whichever process actually runs the steps (the run worker for
/// `fabro run`).
/// Carrying the source form out of the config resolve layer keeps
/// `fabro validate` portable (it never requires env to be set).
///
/// A referenced env var that is unset is a hard error — no fallback to the
/// unresolved source. Reserved `secrets`/`inputs` tokens have no lookup
/// here and surface as a loud
/// A referenced env var or secret that is unset is a hard error — no
/// fallback to the unresolved source. Reserved `inputs` tokens have no
/// lookup here and surface as a loud
/// [`super::interp::ResolveErrorKind::Unavailable`] error rather than
/// passing through as literal text.
pub fn resolve_step_env(
&self,
mut env_lookup: impl FnMut(&str) -> Option<String>,
mut secrets_lookup: impl FnMut(&str) -> Option<String>,
) -> Result<Self, ResolveError> {
let mut resolved = self.clone();
for step in &mut resolved.steps {
visit_prepared_step_strings(step, &mut |value| {
resolve_env_string(value, &mut env_lookup)
resolve_env_string(value, &mut env_lookup, &mut secrets_lookup)
})?;
}
Ok(resolved)
@ -1059,17 +1061,35 @@ impl RunEnvironmentSettings {
}
}
/// Resolve every environment value's `{{ env.* }}` tokens via `lookup`,
/// falling back to the original source string when resolution fails.
#[must_use]
pub fn resolve_env<F>(&self, mut lookup: F) -> HashMap<String, String>
where
F: FnMut(&str) -> Option<String>,
{
self.env
.iter()
.map(|(name, value)| (name.clone(), value.resolve_or_source(&mut lookup)))
.collect()
/// Resolve every environment value's `{{ env.* }}` and `{{ secrets.* }}`
/// tokens via the supplied lookups. Missing env vars retain the historical
/// fallback to the original source string; missing secrets fail closed.
pub fn resolve_env(
&self,
mut env_lookup: impl FnMut(&str) -> Option<String>,
mut secrets_lookup: impl FnMut(&str) -> Option<String>,
) -> Result<HashMap<String, String>, ResolveError> {
let mut resolved = HashMap::with_capacity(self.env.len());
for (name, value) in &self.env {
let mut ctx = ResolveCtx::new()
.with_env(&mut env_lookup)
.with_secrets(&mut secrets_lookup);
let resolved_value = match value.resolve_with(&mut ctx) {
Ok(resolved) => resolved.value,
Err(err) if err.namespace == Namespace::Env => {
#[expect(
clippy::disallowed_methods,
reason = "intentional raw-source fallback preserves existing \
environment variable behavior for run environment values"
)]
let source = value.as_source();
source
}
Err(err) => return Err(err),
};
resolved.insert(name.clone(), resolved_value);
}
Ok(resolved)
}
}
@ -1093,13 +1113,22 @@ mod run_environment_settings_tests {
}
}
fn lookup(
pairs: &'static [(&'static str, &'static str)],
) -> impl Fn(&str) -> Option<String> + Copy {
move |name| {
pairs
.iter()
.find_map(|(key, value)| (*key == name).then(|| (*value).to_string()))
}
}
#[test]
fn resolve_env_substitutes_env_tokens_via_lookup() {
let s = settings(&[("NODE_ENV", "{{ env.NODE_ENV }}"), ("STATIC", "value")]);
let resolved = s.resolve_env(|name| match name {
"NODE_ENV" => Some("test".to_string()),
_ => None,
});
let resolved = s
.resolve_env(lookup(&[("NODE_ENV", "test")]), lookup(&[]))
.unwrap();
assert_eq!(resolved.get("NODE_ENV"), Some(&"test".to_string()));
assert_eq!(resolved.get("STATIC"), Some(&"value".to_string()));
@ -1108,7 +1137,7 @@ mod run_environment_settings_tests {
#[test]
fn resolve_env_falls_back_to_source_when_lookup_fails() {
let s = settings(&[("NODE_ENV", "{{ env.MISSING_NODE_ENV }}")]);
let resolved = s.resolve_env(|_| None);
let resolved = s.resolve_env(lookup(&[]), lookup(&[])).unwrap();
assert_eq!(
resolved.get("NODE_ENV"),
@ -1116,9 +1145,34 @@ mod run_environment_settings_tests {
);
}
#[test]
fn resolve_env_substitutes_secret_tokens_via_lookup() {
let s = settings(&[("API_TOKEN", "Bearer {{ secrets.API_TOKEN }}")]);
let resolved = s
.resolve_env(lookup(&[]), lookup(&[("API_TOKEN", "vault-token")]))
.unwrap();
assert_eq!(
resolved.get("API_TOKEN"),
Some(&"Bearer vault-token".to_string())
);
}
#[test]
fn resolve_env_returns_secret_error_without_source_fallback() {
let s = settings(&[("API_TOKEN", "{{ secrets.MISSING_TOKEN }}")]);
let err = s.resolve_env(lookup(&[]), lookup(&[])).unwrap_err();
assert_eq!(err.namespace, super::Namespace::Secrets);
assert_eq!(err.name, "MISSING_TOKEN");
}
#[test]
fn resolve_env_is_empty_for_empty_settings() {
let s: HashMap<String, String> = settings(&[]).resolve_env(|_| None);
let s: HashMap<String, String> =
settings(&[]).resolve_env(lookup(&[]), lookup(&[])).unwrap();
assert!(s.is_empty());
}
}
@ -1503,45 +1557,50 @@ impl McpServerSettings {
StdDuration::from_secs(self.tool_timeout_secs)
}
/// Resolve `{{ env.* }}` tokens in this server's transport strings
/// (`command`/`args`/`url`/`env`/`headers`) against `env_lookup`,
/// returning a copy with the tokens replaced and every other field
/// preserved.
/// Resolve `{{ env.* }}` and `{{ secrets.* }}` tokens in this server's
/// transport strings (`command`/`args`/`url`/`env`/`headers`) against the
/// supplied lookups, returning a copy with the tokens replaced and every
/// other field preserved.
///
/// This is the late, use-time half of MCP interpolation, the counterpart
/// to [`substitute_mcp_transport`]: `{{ vars.* }}` are substituted
/// earlier, server-side, while `{{ env.* }}` resolve here — in whichever
/// process actually launches the server (the run worker for `fabro run`,
/// the CLI process for `fabro exec`). Carrying the source form out of the
/// config resolve layer keeps `fabro validate` portable (it never requires
/// env to be set).
/// earlier, server-side, while `{{ env.* }}` and `{{ secrets.* }}` resolve
/// here — in whichever process actually launches the server (the run worker
/// for `fabro run`, the CLI process for `fabro exec`). Carrying the source
/// form out of the config resolve layer keeps `fabro validate` portable (it
/// never requires env to be set).
///
/// A referenced env var that is unset is a hard error — no fallback to the
/// unresolved source. Reserved `secrets`/`inputs` tokens have no lookup
/// here and surface as a loud [`ResolveErrorKind::Unavailable`] error
/// rather than passing through as literal text.
/// A referenced env var or secret that is unset is a hard error — no
/// fallback to the unresolved source. Reserved `inputs` tokens have no
/// lookup here and surface as a loud [`ResolveErrorKind::Unavailable`]
/// error rather than passing through as literal text.
pub fn resolve_transport_env(
&self,
mut env_lookup: impl FnMut(&str) -> Option<String>,
mut secrets_lookup: impl FnMut(&str) -> Option<String>,
) -> Result<Self, ResolveError> {
let mut resolved = self.clone();
visit_mcp_transport_strings(&mut resolved.transport, &mut |value| {
resolve_env_string(value, &mut env_lookup)
resolve_env_string(value, &mut env_lookup, &mut secrets_lookup)
})?;
Ok(resolved)
}
}
/// Resolve `{{ env.* }}` tokens in one MCP transport string. A literal value
/// (no tokens) round-trips unchanged.
/// Resolve `{{ env.* }}` and `{{ secrets.* }}` tokens in one run-boundary
/// string. A literal value (no tokens) round-trips unchanged.
fn resolve_env_string(
value: &mut String,
env_lookup: &mut impl FnMut(&str) -> Option<String>,
secrets_lookup: &mut impl FnMut(&str) -> Option<String>,
) -> Result<(), ResolveError> {
if !value.contains("{{") {
return Ok(());
}
*value = InterpString::parse(value).resolve(&mut *env_lookup)?.value;
let mut ctx = ResolveCtx::new()
.with_env(&mut *env_lookup)
.with_secrets(&mut *secrets_lookup);
*value = InterpString::parse(value).resolve_with(&mut ctx)?.value;
Ok(())
}
@ -1562,6 +1621,16 @@ mod resolve_transport_env_tests {
}
}
fn secret_lookup(
pairs: &'static [(&'static str, &'static str)],
) -> impl Fn(&str) -> Option<String> + Copy {
move |name| {
pairs
.iter()
.find_map(|(key, value)| (*key == name).then(|| (*value).to_string()))
}
}
#[test]
fn literal_transport_passes_through() {
let settings = McpServerSettings {
@ -1573,7 +1642,9 @@ mod resolve_transport_env_tests {
..McpServerSettings::default()
};
let resolved = settings.resolve_transport_env(env_lookup(&[])).unwrap();
let resolved = settings
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
.unwrap();
let McpTransport::Stdio { command, env } = resolved.transport else {
panic!("expected stdio transport");
@ -1597,10 +1668,13 @@ mod resolve_transport_env_tests {
};
let resolved = settings
.resolve_transport_env(env_lookup(&[
("SERVER_PATH", "/srv/mcp.py"),
("GEMINI_API_KEY", "real-key"),
]))
.resolve_transport_env(
env_lookup(&[
("SERVER_PATH", "/srv/mcp.py"),
("GEMINI_API_KEY", "real-key"),
]),
secret_lookup(&[]),
)
.unwrap();
let McpTransport::Stdio { command, env } = resolved.transport else {
@ -1632,10 +1706,10 @@ mod resolve_transport_env_tests {
};
let resolved = settings
.resolve_transport_env(env_lookup(&[
("MCP_HOST", "mcp.example"),
("MCP_TOKEN", "abc123"),
]))
.resolve_transport_env(
env_lookup(&[("MCP_HOST", "mcp.example"), ("MCP_TOKEN", "abc123")]),
secret_lookup(&[]),
)
.unwrap();
let McpTransport::Http { url, headers, .. } = resolved.transport else {
@ -1662,7 +1736,9 @@ mod resolve_transport_env_tests {
..McpServerSettings::default()
};
let err = settings.resolve_transport_env(env_lookup(&[])).unwrap_err();
let err = settings
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
.unwrap_err();
assert_eq!(err.namespace, Namespace::Env);
assert_eq!(err.name, "GEMINI_API_KEY");
@ -1670,7 +1746,78 @@ mod resolve_transport_env_tests {
}
#[test]
fn reserved_secret_token_is_unavailable_not_leaked() {
fn stdio_command_and_env_resolve_secret_tokens() {
let settings = McpServerSettings {
name: "vaulted".to_string(),
transport: McpTransport::Stdio {
command: vec![
"{{ secrets.SERVER_BIN }}".to_string(),
"--token".to_string(),
"{{ secrets.API_TOKEN }}".to_string(),
],
env: HashMap::from([(
"API_TOKEN".to_string(),
"{{ secrets.API_TOKEN }}".to_string(),
)]),
},
..McpServerSettings::default()
};
let resolved = settings
.resolve_transport_env(
env_lookup(&[]),
secret_lookup(&[("SERVER_BIN", "/srv/mcp"), ("API_TOKEN", "vault-token")]),
)
.unwrap();
let McpTransport::Stdio { command, env } = resolved.transport else {
panic!("expected stdio transport");
};
assert_eq!(command, vec![
"/srv/mcp".to_string(),
"--token".to_string(),
"vault-token".to_string()
]);
assert_eq!(
env.get("API_TOKEN").map(String::as_str),
Some("vault-token")
);
}
#[test]
fn http_url_and_headers_resolve_secret_tokens() {
let settings = McpServerSettings {
name: "remote".to_string(),
transport: McpTransport::Http {
protocol: McpHttpProtocol::default(),
url: "https://{{ secrets.MCP_HOST }}/mcp".to_string(),
headers: HashMap::from([(
"Authorization".to_string(),
"Bearer {{ secrets.MCP_TOKEN }}".to_string(),
)]),
},
..McpServerSettings::default()
};
let resolved = settings
.resolve_transport_env(
env_lookup(&[]),
secret_lookup(&[("MCP_HOST", "mcp.example"), ("MCP_TOKEN", "vault-token")]),
)
.unwrap();
let McpTransport::Http { url, headers, .. } = resolved.transport else {
panic!("expected http transport");
};
assert_eq!(url, "https://mcp.example/mcp");
assert_eq!(
headers.get("Authorization").map(String::as_str),
Some("Bearer vault-token")
);
}
#[test]
fn missing_secret_token_is_secret_error() {
let settings = McpServerSettings {
name: "vaulted".to_string(),
transport: McpTransport::Stdio {
@ -1683,10 +1830,13 @@ mod resolve_transport_env_tests {
..McpServerSettings::default()
};
let err = settings.resolve_transport_env(env_lookup(&[])).unwrap_err();
let err = settings
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
.unwrap_err();
assert_eq!(err.namespace, Namespace::Secrets);
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
assert_eq!(err.name, "API_KEY");
assert_eq!(err.kind, ResolveErrorKind::Missing);
}
}
@ -1707,6 +1857,16 @@ mod resolve_step_env_tests {
}
}
fn secret_lookup(
pairs: &'static [(&'static str, &'static str)],
) -> impl Fn(&str) -> Option<String> + Copy {
move |name| {
pairs
.iter()
.find_map(|(key, value)| (*key == name).then(|| (*value).to_string()))
}
}
fn script_step(script: &str, env: HashMap<String, String>) -> PreparedStep {
PreparedStep {
run: PreparedStepRun::Script {
@ -1735,7 +1895,9 @@ mod resolve_step_env_tests {
timeout_ms: 1_000,
};
let resolved = settings.resolve_step_env(env_lookup(&[])).unwrap();
let resolved = settings
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
.unwrap();
assert_eq!(resolved.steps[0].to_shell_command(), "echo hello");
assert_eq!(
@ -1758,7 +1920,7 @@ mod resolve_step_env_tests {
};
let resolved = settings
.resolve_step_env(env_lookup(&[("REGION", "us-east-1")]))
.resolve_step_env(env_lookup(&[("REGION", "us-east-1")]), secret_lookup(&[]))
.unwrap();
assert_eq!(
@ -1778,10 +1940,10 @@ mod resolve_step_env_tests {
};
let resolved = settings
.resolve_step_env(env_lookup(&[
("REGION", "us-east-1"),
("DEPLOY_TOKEN", "secret-token"),
]))
.resolve_step_env(
env_lookup(&[("REGION", "us-east-1"), ("DEPLOY_TOKEN", "secret-token")]),
secret_lookup(&[]),
)
.unwrap();
assert_eq!(resolved.steps[0].to_shell_command(), "deploy us-east-1");
@ -1801,7 +1963,10 @@ mod resolve_step_env_tests {
};
let resolved = settings
.resolve_step_env(env_lookup(&[("MESSAGE", "hello world")]))
.resolve_step_env(
env_lookup(&[("MESSAGE", "hello world")]),
secret_lookup(&[]),
)
.unwrap();
let shell = resolved.steps[0].to_shell_command();
@ -1826,7 +1991,10 @@ mod resolve_step_env_tests {
};
let resolved = settings
.resolve_step_env(|name| (name == "USER_INPUT").then(|| malicious.to_string()))
.resolve_step_env(
|name| (name == "USER_INPUT").then(|| malicious.to_string()),
secret_lookup(&[]),
)
.unwrap();
let shell = resolved.steps[0].to_shell_command();
@ -1856,7 +2024,9 @@ mod resolve_step_env_tests {
timeout_ms: 1_000,
};
let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err();
let err = settings
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
.unwrap_err();
assert_eq!(err.namespace, Namespace::Env);
assert_eq!(err.name, "REGION");
@ -1873,7 +2043,9 @@ mod resolve_step_env_tests {
timeout_ms: 1_000,
};
let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err();
let err = settings
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
.unwrap_err();
assert_eq!(err.namespace, Namespace::Env);
assert_eq!(err.name, "DEPLOY_TOKEN");
@ -1881,7 +2053,45 @@ mod resolve_step_env_tests {
}
#[test]
fn reserved_secret_token_is_unavailable_not_leaked() {
fn script_command_and_env_resolve_secret_tokens() {
let settings = RunPrepareSettings {
steps: vec![
script_step(
"deploy {{ secrets.REGION }} && echo done",
HashMap::from([(
"TOKEN".to_string(),
"{{ secrets.DEPLOY_TOKEN }}".to_string(),
)]),
),
command_step(&["notify", "{{ secrets.MESSAGE }}"], HashMap::new()),
],
timeout_ms: 1_000,
};
let resolved = settings
.resolve_step_env(
env_lookup(&[]),
secret_lookup(&[
("REGION", "us-east-1"),
("DEPLOY_TOKEN", "vault-token"),
("MESSAGE", "hello world"),
]),
)
.unwrap();
assert_eq!(
resolved.steps[0].to_shell_command(),
"deploy us-east-1 && echo done"
);
assert_eq!(
resolved.steps[0].env.get("TOKEN").map(String::as_str),
Some("vault-token")
);
assert_eq!(resolved.steps[1].to_shell_command(), "notify 'hello world'");
}
#[test]
fn missing_secret_token_is_secret_error() {
let settings = RunPrepareSettings {
steps: vec![script_step(
"echo hi",
@ -1890,10 +2100,13 @@ mod resolve_step_env_tests {
timeout_ms: 1_000,
};
let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err();
let err = settings
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
.unwrap_err();
assert_eq!(err.namespace, Namespace::Secrets);
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
assert_eq!(err.name, "API_KEY");
assert_eq!(err.kind, ResolveErrorKind::Missing);
}
}

View file

@ -85,3 +85,4 @@ httpmock = "0.8"
fabro-macros = { path = "../fabro-macros" }
fabro-test = { workspace = true }
fabro-types = { path = "../fabro-types", features = ["test-support"] }
shlex = "1"

View file

@ -432,14 +432,14 @@ mod tests {
use chrono::{Local, TimeZone, Utc};
use fabro_config::{
ReplaceMap, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, RunPullRequestLayer,
WorkflowSettingsBuilder,
PrepareStep, ReplaceMap, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer,
RunPrepareLayer, RunPullRequestLayer, WorkflowSettingsBuilder,
};
use fabro_graphviz::graph::AttrValue;
use fabro_store::Database;
use fabro_types::settings::InterpString;
use fabro_types::settings::run::RunMode;
use fabro_types::{WorkflowSettings, fixtures, test_support};
use fabro_types::{EventBody, WorkflowSettings, fixtures, test_support};
use fabro_util::error::collect_chain;
use fabro_validate::Severity;
use object_store::local::LocalFileSystem;
@ -1389,6 +1389,91 @@ mod tests {
assert!(created.run_dir.is_dir());
}
#[tokio::test]
async fn create_persists_secret_tokens_in_run_created_settings_source_form() {
let dir = tempfile::tempdir().unwrap();
let storage_root = dir.path().join("storage");
let store = memory_store();
let created = create(
&store,
CreateRunInput {
workflow: WorkflowInput::DotSource {
source: MINIMAL_DOT.to_string(),
base_dir: None,
},
settings: settings_from_run_layer(RunLayer {
prepare: Some(RunPrepareLayer {
steps: vec![PrepareStep {
script: None,
command: Some(vec![
InterpString::parse("deploy"),
InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"),
]),
env: HashMap::from([(
"DEPLOY_TOKEN".to_string(),
InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"),
)]),
}],
timeout: None,
}),
execution: Some(RunExecutionLayer {
mode: Some(RunMode::DryRun),
..RunExecutionLayer::default()
}),
..RunLayer::default()
}),
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
workflow_slug: Some("secret-source".to_string()),
workflow_path: None,
workflow_bundle: None,
submitted_manifest_bytes: None,
run_id: Some(fixtures::RUN_1),
title: None,
automation: None,
git: None,
fork_source_ref: None,
parent_id: None,
provenance: test_support::test_run_provenance(),
configured_providers: Vec::new(),
web_url: None,
},
storage_root,
test_catalog(),
)
.await
.unwrap();
let run_store = store.open_run(&created.run_id).await.unwrap();
let events = run_store.list_events().await.unwrap();
let run_created = events
.iter()
.find_map(|event| match &event.event.body {
EventBody::RunCreated(props) => Some(props),
_ => None,
})
.expect("run.created event should be persisted");
let step = run_created
.settings
.run
.prepare
.steps
.first()
.expect("prepare step should be persisted");
let fabro_types::settings::run::PreparedStepRun::Command { command } = &step.run else {
panic!("expected command prepare step");
};
assert_eq!(command, &vec![
"deploy".to_string(),
"{{ secrets.DEPLOY_TOKEN }}".to_string()
]);
assert_eq!(
step.env.get("DEPLOY_TOKEN").map(String::as_str),
Some("{{ secrets.DEPLOY_TOKEN }}")
);
}
#[tokio::test]
async fn create_persists_submitter_source_directory_from_request_cwd() {
let dir = tempfile::tempdir().unwrap();

View file

@ -10,7 +10,7 @@ use fabro_mcp::config::McpServerSettings;
use fabro_model::{Catalog, FallbackTarget, ProviderId};
use fabro_sandbox::daytona::DaytonaConfig;
use fabro_sandbox::from_environment::{
daytona_config_from_environment, docker_config_from_environment,
daytona_config_from_environment, docker_config_from_environment_with_secrets,
local_working_directory_from_environment,
};
use fabro_sandbox::{DockerSandboxOptions, SandboxSpec};
@ -373,12 +373,20 @@ impl RunSession {
let configured =
configured_providers_for_start(services.vault.as_ref(), Arc::clone(&catalog)).await;
let llm = resolve_start_llm(catalog.as_ref(), &configured, resolved)?;
let vault_guard = match services.vault.as_ref() {
Some(vault) => Some(vault.read().await),
None => None,
};
let mcp_servers = resolved
.agent
.mcps
.iter()
.map(|(key, entry)| match entry {
ResolvedMcpEntry::Resolved(server) => runtime_mcp_server(server, process_env_var),
ResolvedMcpEntry::Resolved(server) => {
runtime_mcp_server(server, process_env_var, |name| {
vault_token_lookup(vault_guard.as_deref(), name)
})
}
// References must be resolved to concrete servers before the run
// spec is persisted (server-side run-preparation pass). Reaching
// worker startup with an unresolved reference is an invariant
@ -409,19 +417,17 @@ impl RunSession {
SandboxSpec::Local { working_directory }
}
SandboxProviderKind::Docker => SandboxSpec::Docker {
config: resolve_docker_config(resolved),
config: resolve_docker_config(resolved, |name| {
vault_token_lookup(vault_guard.as_deref(), name)
})?,
github_app: services.github_app.clone(),
run_id: Some(record.run_id),
clone_origin_url: record.repo_origin_url().map(str::to_string),
clone_branch: record.base_branch().map(str::to_string),
},
SandboxProviderKind::Daytona => {
let api_key = match &services.vault {
Some(v) => v
.read()
.await
.get(EnvVars::DAYTONA_API_KEY)
.map(str::to_string),
let api_key = match vault_guard.as_deref() {
Some(vault) => vault.get(EnvVars::DAYTONA_API_KEY).map(str::to_string),
None => None,
};
SandboxSpec::Daytona {
@ -435,7 +441,12 @@ impl RunSession {
}
};
let toml_env = resolved.environment.resolve_env(process_env_var);
let toml_env = resolved
.environment
.resolve_env(process_env_var, |name| {
vault_token_lookup(vault_guard.as_deref(), name)
})
.map_err(|err| Error::engine_with_source("failed to resolve run environment", err))?;
let github_permissions: Option<HashMap<String, String>> =
(!services.github_permissions.is_empty()).then(|| services.github_permissions.clone());
let sandbox_env = SandboxEnvSpec {
@ -452,6 +463,10 @@ impl RunSession {
};
let pr_config = resolved.pull_request.clone();
let setup_commands = runtime_setup_commands(&resolved.prepare, process_env_var, |name| {
vault_token_lookup(vault_guard.as_deref(), name)
})?;
drop(vault_guard);
Ok(Self {
cancel_token: services.cancel_token,
@ -471,7 +486,7 @@ impl RunSession {
steering_hub: services.steering_hub,
on_node: services.on_node,
lifecycle: LifecycleOptions {
setup_commands: runtime_setup_commands(&resolved.prepare)?,
setup_commands,
setup_command_timeout_ms: resolved.prepare.timeout_ms,
},
hooks: fabro_hooks::HookSettings {
@ -550,6 +565,10 @@ fn process_env_var(name: &str) -> Option<String> {
std::env::var(name).ok()
}
fn vault_token_lookup(vault: Option<&Vault>, name: &str) -> Option<String> {
vault.and_then(|vault| fabro_auth::vault_get_token(vault, name).ok().flatten())
}
async fn load_accepted_run_definition(
run_store: &RunStoreHandle,
blob_id: fabro_types::RunBlobId,
@ -574,8 +593,16 @@ fn resolve_daytona_config(settings: &ResolvedRunSettings) -> DaytonaConfig {
daytona_config_from_environment(&settings.environment, !settings.clone.enabled)
}
fn resolve_docker_config(settings: &ResolvedRunSettings) -> DockerSandboxOptions {
docker_config_from_environment(&settings.environment, !settings.clone.enabled)
fn resolve_docker_config(
settings: &ResolvedRunSettings,
secrets_lookup: impl FnMut(&str) -> Option<String>,
) -> Result<DockerSandboxOptions, Error> {
docker_config_from_environment_with_secrets(
&settings.environment,
!settings.clone.enabled,
secrets_lookup,
)
.map_err(|err| Error::engine_with_source("failed to resolve Docker environment config", err))
}
fn resolve_start_llm(
@ -680,46 +707,51 @@ impl ModelRegistry for CatalogModelRegistry<'_> {
}
/// Build the launch-time MCP config from resolved settings, resolving any
/// `{{ env.* }}` tokens in the transport (`command`/`url`/`env`/`headers`)
/// against the worker process environment — the run boundary where the MCP is
/// actually launched.
/// `{{ env.* }}` and `{{ secrets.* }}` tokens in the transport
/// (`command`/`url`/`env`/`headers`) against the worker process environment and
/// vault — the run boundary where the MCP is actually launched.
///
/// The resolution itself lives on the type
/// ([`McpServerSettings::resolve_transport_env`]) so `fabro run` (here) and
/// `fabro exec` share one resolver; this wrapper just adds the server name to
/// the error. MCP transport strings are carried in source form out of the
/// config resolve layer so `fabro validate` stays portable (it never requires
/// env to be set), and a referenced env var that is unset is a hard error —
/// no fallback to the unresolved source.
/// env to be set), and a referenced env var or secret that is unset is a hard
/// error — no fallback to the unresolved source.
fn runtime_mcp_server(
settings: &ResolvedMcpServerSettings,
env_lookup: impl FnMut(&str) -> Option<String>,
secrets_lookup: impl FnMut(&str) -> Option<String>,
) -> Result<McpServerSettings, Error> {
settings.resolve_transport_env(env_lookup).map_err(|err| {
Error::engine_with_source(
format!("failed to resolve MCP server {:?}", settings.name),
err,
)
})
settings
.resolve_transport_env(env_lookup, secrets_lookup)
.map_err(|err| {
Error::engine_with_source(
format!("failed to resolve MCP server {:?}", settings.name),
err,
)
})
}
/// Build the launch-time setup (prepare) commands from resolved settings,
/// resolving any `{{ env.* }}` tokens in each step's command and per-step env
/// against the worker process environment — the run boundary where the steps
/// actually run.
/// resolving any `{{ env.* }}` and `{{ secrets.* }}` tokens in each step's
/// command and per-step env against the worker process environment and vault —
/// the run boundary where the steps actually run.
///
/// The resolution itself lives on the type
/// ([`ResolvedRunPrepareSettings::resolve_step_env`]) so prepare-step env
/// resolution shares one resolver with the rest of the run-boundary
/// interpolation. Prepare-step commands and env are carried in source form out
/// of the config resolve layer so `fabro validate` stays portable (it never
/// requires env to be set), and a referenced env var that is unset is a hard
/// error — no fallback to the unresolved source.
/// requires env to be set), and a referenced env var or secret that is unset is
/// a hard error — no fallback to the unresolved source.
fn runtime_setup_commands(
prepare: &ResolvedRunPrepareSettings,
env_lookup: impl FnMut(&str) -> Option<String>,
secrets_lookup: impl FnMut(&str) -> Option<String>,
) -> Result<Vec<SetupCommand>, Error> {
let resolved = prepare
.resolve_step_env(process_env_var)
.resolve_step_env(env_lookup, secrets_lookup)
.map_err(|err| Error::engine_with_source("failed to resolve prepare step", err))?;
Ok(resolved
.steps
@ -1095,11 +1127,15 @@ mod tests {
RunEnvironmentLayer, RunExecutionLayer, RunLayer, StickyMap, WorkflowSettingsBuilder,
};
use fabro_store::Database;
use fabro_types::settings::run::{McpTransport as ResolvedMcpTransport, RunMode};
use fabro_types::settings::run::{
McpTransport as ResolvedMcpTransport, PreparedStep, PreparedStepRun, RunMode,
RunPrepareSettings,
};
use fabro_types::settings::{InterpString, ModelRef};
use fabro_types::{
BilledModelUsage, ManifestPath, StageTiming, WorkflowSettings, fixtures, test_support,
};
use fabro_vault::SecretType;
use object_store::memory::InMemory;
use super::*;
@ -1284,7 +1320,11 @@ reasoning = false
..RunLayer::default()
});
assert!(resolve_docker_config(&settings.run).skip_clone);
assert!(
resolve_docker_config(&settings.run, |_| None)
.unwrap()
.skip_clone
);
assert!(resolve_daytona_config(&settings.run).skip_clone);
}
@ -1302,7 +1342,7 @@ reasoning = false
..ResolvedMcpServerSettings::default()
};
let err = runtime_mcp_server(&settings, |_| None).unwrap_err();
let err = runtime_mcp_server(&settings, |_| None, |_| None).unwrap_err();
assert_eq!(
err.to_string(),
@ -1313,6 +1353,261 @@ reasoning = false
assert!(causes[0].contains("GEMINI_API_KEY"));
}
#[test]
fn runtime_setup_command_env_resolves_secret_from_vault() {
let vault = token_vault("DEPLOY_TOKEN", "vault-token");
let prepare = prepare_with_step(script_step(
"echo ready",
HashMap::from([(
"DEPLOY_TOKEN".to_string(),
"{{ secrets.DEPLOY_TOKEN }}".to_string(),
)]),
));
let commands =
runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)).unwrap();
assert_eq!(commands.len(), 1);
assert_eq!(
commands[0].env.get("DEPLOY_TOKEN").map(String::as_str),
Some("vault-token")
);
}
#[test]
fn runtime_setup_command_secret_argv_is_resolved_before_shell_quoting() {
let malicious = "x'; touch PWNED; echo '";
let vault = token_vault("USER_INPUT", malicious);
let prepare = prepare_with_step(command_step(
&["echo", "{{ secrets.USER_INPUT }}"],
HashMap::new(),
));
let commands =
runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)).unwrap();
let tokens =
shlex::split(&commands[0].command).expect("resolved command should remain valid shell");
assert_eq!(tokens, vec!["echo".to_string(), malicious.to_string()]);
assert_eq!(
tokens.len(),
2,
"injected shell syntax leaked extra tokens: {}",
commands[0].command
);
}
#[test]
fn runtime_mcp_server_env_resolves_secret_from_vault() {
let vault = token_vault("MCP_TOKEN", "vault-token");
let settings = ResolvedMcpServerSettings {
name: "vaulted".to_string(),
transport: ResolvedMcpTransport::Stdio {
command: vec!["mcp-server".to_string()],
env: HashMap::from([(
"MCP_TOKEN".to_string(),
"{{ secrets.MCP_TOKEN }}".to_string(),
)]),
},
..ResolvedMcpServerSettings::default()
};
let resolved =
runtime_mcp_server(&settings, |_| None, vault_secret_lookup(&vault)).unwrap();
let ResolvedMcpTransport::Stdio { env, .. } = resolved.transport else {
panic!("expected stdio transport");
};
assert_eq!(
env.get("MCP_TOKEN").map(String::as_str),
Some("vault-token")
);
}
#[test]
fn runtime_setup_command_missing_secret_fails_closed() {
let vault = temp_vault(&[]);
let prepare = prepare_with_step(command_step(
&["deploy", "{{ secrets.DEPLOY_TOKEN }}"],
HashMap::new(),
));
let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault))
else {
panic!("missing secret should fail setup command resolution");
};
assert_eq!(
err.to_string(),
"Engine error: failed to resolve prepare step"
);
let causes = err.causes();
assert_eq!(causes.len(), 1);
assert!(causes[0].contains("DEPLOY_TOKEN"));
}
#[test]
fn runtime_setup_command_oauth_secret_fails_closed() {
let vault = temp_vault(&[("DEPLOY_TOKEN", "{}", SecretType::Oauth)]);
let prepare = prepare_with_step(script_step(
"echo ready",
HashMap::from([(
"DEPLOY_TOKEN".to_string(),
"{{ secrets.DEPLOY_TOKEN }}".to_string(),
)]),
));
let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault))
else {
panic!("OAuth secret should fail setup command resolution");
};
assert_eq!(
err.to_string(),
"Engine error: failed to resolve prepare step"
);
assert!(err.causes()[0].contains("DEPLOY_TOKEN"));
}
#[test]
fn runtime_setup_command_file_secret_fails_closed() {
let vault = temp_vault(&[(EnvVars::GITHUB_APP_PRIVATE_KEY, "pem", SecretType::File)]);
let prepare = prepare_with_step(script_step(
"echo ready",
HashMap::from([(
"GITHUB_APP_PRIVATE_KEY".to_string(),
"{{ secrets.GITHUB_APP_PRIVATE_KEY }}".to_string(),
)]),
));
let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault))
else {
panic!("file secret should fail setup command resolution");
};
assert_eq!(
err.to_string(),
"Engine error: failed to resolve prepare step"
);
assert!(err.causes()[0].contains("GITHUB_APP_PRIVATE_KEY"));
}
#[tokio::test]
async fn run_session_new_resolves_secret_tokens_from_vault_at_boundary() {
let temp = tempfile::tempdir().unwrap();
let (storage_root, _run_dir) = storage_root_and_run_dir(&temp);
let mut settings = settings_from_run_layer(RunLayer {
execution: Some(RunExecutionLayer {
mode: Some(RunMode::DryRun),
..RunExecutionLayer::default()
}),
..RunLayer::default()
});
settings.run.environment.env.insert(
"API_TOKEN".to_string(),
InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"),
);
settings.run.prepare = prepare_with_step(command_step(
&["deploy", "{{ secrets.DEPLOY_TOKEN }}"],
HashMap::from([(
"DEPLOY_TOKEN".to_string(),
"{{ secrets.DEPLOY_TOKEN }}".to_string(),
)]),
));
settings.run.agent.mcps.insert(
"vaulted".to_string(),
ResolvedMcpEntry::Resolved(ResolvedMcpServerSettings {
name: "vaulted".to_string(),
transport: ResolvedMcpTransport::Stdio {
command: vec!["mcp-server".to_string()],
env: HashMap::from([(
"MCP_TOKEN".to_string(),
"{{ secrets.DEPLOY_TOKEN }}".to_string(),
)]),
},
..ResolvedMcpServerSettings::default()
}),
);
let (persisted, store) =
persisted_workflow_with_settings(MINIMAL_DOT, &storage_root, settings).await;
let emitter = Arc::new(Emitter::new(fixtures::RUN_1));
let registry = Arc::new(test_registry());
let vault = Arc::new(AsyncRwLock::new(token_vault("DEPLOY_TOKEN", "vault-token")));
let session = RunSession::new(&persisted, StartServices {
vault: Some(vault),
..test_start_services(&store, &storage_root, emitter, registry).await
})
.await
.unwrap();
assert_eq!(
session
.sandbox_env
.toml_env
.get("API_TOKEN")
.map(String::as_str),
Some("vault-token")
);
assert_eq!(
session.lifecycle.setup_commands[0]
.env
.get("DEPLOY_TOKEN")
.map(String::as_str),
Some("vault-token")
);
let setup_command = &session.lifecycle.setup_commands[0].command;
assert!(!setup_command.contains("{{ secrets.DEPLOY_TOKEN }}"));
assert_eq!(
shlex::split(setup_command).expect("setup command should be valid shell"),
vec!["deploy".to_string(), "vault-token".to_string()]
);
let ResolvedMcpTransport::Stdio { env, .. } = &session.llm.mcp_servers[0].transport else {
panic!("expected stdio MCP transport");
};
assert_eq!(
env.get("MCP_TOKEN").map(String::as_str),
Some("vault-token")
);
}
#[tokio::test]
async fn run_session_new_missing_secret_fails_startup() {
let temp = tempfile::tempdir().unwrap();
let (storage_root, _run_dir) = storage_root_and_run_dir(&temp);
let mut settings = settings_from_run_layer(RunLayer {
execution: Some(RunExecutionLayer {
mode: Some(RunMode::DryRun),
..RunExecutionLayer::default()
}),
..RunLayer::default()
});
settings.run.prepare = prepare_with_step(command_step(
&["deploy", "{{ secrets.DEPLOY_TOKEN }}"],
HashMap::new(),
));
let (persisted, store) =
persisted_workflow_with_settings(MINIMAL_DOT, &storage_root, settings).await;
let emitter = Arc::new(Emitter::new(fixtures::RUN_1));
let registry = Arc::new(test_registry());
let vault = Arc::new(AsyncRwLock::new(temp_vault(&[])));
let Err(err) = RunSession::new(&persisted, StartServices {
vault: Some(vault),
..test_start_services(&store, &storage_root, emitter, registry).await
})
.await
else {
panic!("missing secret should fail run startup");
};
assert_eq!(
err.to_string(),
"Engine error: failed to resolve prepare step"
);
assert!(err.causes()[0].contains("DEPLOY_TOKEN"));
}
#[test]
fn runtime_docker_config_maps_environment_hints() {
let settings = settings_from_run_layer(RunLayer {
@ -1339,7 +1634,7 @@ reasoning = false
..RunLayer::default()
});
let config = resolve_docker_config(&settings.run);
let config = resolve_docker_config(&settings.run, |_| None).unwrap();
assert_eq!(config.image, "ubuntu:24.04");
assert_eq!(config.cpu_quota, Some(400_000));
@ -1381,7 +1676,11 @@ reasoning = false
assert_eq!(git.meta_branch, None);
}
async fn persisted_workflow(dot: &str, storage_root: &Path) -> (Persisted, Arc<Database>) {
async fn persisted_workflow_with_settings(
dot: &str,
storage_root: &Path,
settings: WorkflowSettings,
) -> (Persisted, Arc<Database>) {
let store = memory_store();
let created = crate::operations::create(
&store,
@ -1390,13 +1689,7 @@ reasoning = false
source: dot.to_string(),
base_dir: None,
},
settings: settings_from_run_layer(RunLayer {
execution: Some(RunExecutionLayer {
mode: Some(RunMode::DryRun),
..RunExecutionLayer::default()
}),
..RunLayer::default()
}),
settings,
vars: std::collections::HashMap::new(),
cwd: storage_root
.parent()
@ -1424,6 +1717,21 @@ reasoning = false
(created.persisted, store)
}
async fn persisted_workflow(dot: &str, storage_root: &Path) -> (Persisted, Arc<Database>) {
persisted_workflow_with_settings(
dot,
storage_root,
settings_from_run_layer(RunLayer {
execution: Some(RunExecutionLayer {
mode: Some(RunMode::DryRun),
..RunExecutionLayer::default()
}),
..RunLayer::default()
}),
)
.await
}
fn test_registry() -> HandlerRegistry {
let mut registry = HandlerRegistry::new(Box::new(StartHandler));
registry.register("start", Box::new(StartHandler));
@ -1459,6 +1767,48 @@ reasoning = false
}
}
fn temp_vault(entries: &[(&str, &str, SecretType)]) -> Vault {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
for (name, value, secret_type) in entries {
vault.set(name, value, *secret_type, None).unwrap();
}
vault
}
fn token_vault(name: &str, value: &str) -> Vault {
temp_vault(&[(name, value, SecretType::Token)])
}
fn vault_secret_lookup(vault: &Vault) -> impl FnMut(&str) -> Option<String> + '_ {
move |name| vault_token_lookup(Some(vault), name)
}
fn prepare_with_step(step: PreparedStep) -> RunPrepareSettings {
RunPrepareSettings {
steps: vec![step],
timeout_ms: 1_000,
}
}
fn script_step(script: &str, env: HashMap<String, String>) -> PreparedStep {
PreparedStep {
run: PreparedStepRun::Script {
script: script.to_string(),
},
env,
}
}
fn command_step(command: &[&str], env: HashMap<String, String>) -> PreparedStep {
PreparedStep {
run: PreparedStepRun::Command {
command: command.iter().map(|value| (*value).to_string()).collect(),
},
env,
}
}
use crate::test_support::{mark_run_running, test_usage};
async fn append_completed_stage(