scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Resolve run.prepare.steps env and interpolation at the run boundary (#530)

Browse source 
## What

Per-step environment in `run.prepare.steps[].env` was parsed and then
**dropped** before it reached the resolved run settings, so prepare
steps could never see their declared env. This PR carries that env all
the way through to the executor, resolves prepare-step interpolation at
the run boundary, and fixes an argv-quoting bug.

Three things:

1. **Per-step env is carried through.** `RunPrepareSettings` now holds
`steps: Vec<PreparedStep>` (command plus per-step `env`) instead of a
flat `commands: Vec<String>`. The per-step env reaches `exec_command`,
which already accepts per-command env vars, and is merged on top of the
base sandbox environment.
2. **Interpolation resolves at the run boundary.** Prepare-step
`script`/`command` and per-step `env` values are carried in source form
out of the portable config resolve layer (so `fabro validate` stays
portable and never requires env to be set). Their `{{ env.* }}` tokens
resolve in the process that actually runs the steps, via
`RunPrepareSettings::resolve_step_env` — mirroring the existing MCP
transport env resolution. A missing env var is a **hard error**
(fail-closed); there is no fallback to the unresolved literal.
3. **Argv is shell-quoted.** Argv-style prepare steps were assembled
with `join(" ")`, so an argument containing spaces or quotes was
re-split by the shell. They are now shell-quoted per element with the
shared `shell_quote()` helper. `script` steps stay verbatim because they
are raw shell snippets.

## How

- `RunPrepareSettings.commands: Vec<String>` becomes
`RunPrepareSettings.steps: Vec<PreparedStep>` where `PreparedStep {
command, env }`. The server-side `{{ vars.* }}` substitution pass now
walks each step's command and env.
- New `RunPrepareSettings::resolve_step_env(env_lookup)` resolves `{{
env.* }}` in each step's command and env values, returning a hard error
on a missing var (and a loud `Unavailable` error for reserved
`secrets`/`inputs` tokens).
- The run boundary (`fabro_workflow::operations::start`) gains
`runtime_setup_commands`, the prepare-step counterpart to
`runtime_mcp_server`. `LifecycleOptions` now carries `Vec<SetupCommand>`
(command + env), and the initialize phase passes each step's env to
`exec_command`.
- `resolve_prepare` shell-quotes each argv element and carries per-step
env in source form. The stale lint suppression on the resolved fields is
rewritten to describe the deliberate source preservation that now
resolves at the run boundary.
- The shell-quoting helper moves to a shared `fabro_util::shell` module
(backed by `shlex`); `fabro_sandbox::shell_quote` delegates to it so the
config resolve layer and sandbox code share one audited implementation.
- The OpenAPI `RunPrepareSettings` schema and the generated TypeScript
client are updated to the new `steps`/`PreparedStep` shape.

## Testing

- `cargo build --workspace`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D
warnings`
- `cargo nextest run` for `fabro-util`, `fabro-types`, `fabro-config`,
`fabro-sandbox`, `fabro-api`, `fabro-workflow`, `fabro-server`,
`fabro-cli` (provider keys stripped) — all green.
- `cd lib/packages/fabro-api-client && bun run typecheck` — clean.

New tests cover: per-step env carried through resolution; script/command
+ env resolved at the run boundary; a missing env var is a hard error
(in both the command and a per-step env value); reserved `secrets`
tokens surface as `Unavailable`; argv elements are shell-quoted (an arg
with spaces/quotes is correctly quoted) while a `script` stays verbatim;
and an end-to-end check that per-step env reaches the executed setup
command (with a negative control proving the success is attributable to
the per-step env).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-07-01 10:31:26 -04:00 • committed by GitHub
parent c631ce557b
commit 0244736b05
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
33 changed files with 854 additions and 110 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
Cargo.lock generated
View file

@ -2510,8 +2510,8 @@ dependencies = [
"csv",
"fabro-config",
"fabro-options-metadata",
"fabro-util",
"quick-xml 0.36.2",
"shlex",
"tempfile",
"toml_edit",
"tracing-subscriber",
@ -2881,7 +2881,6 @@ dependencies = [
"serde",
"serde_json",
"sha2 0.10.9",
"shlex",
"strum 0.28.0",
"tar",
"tempfile",
@ -3158,6 +3157,7 @@ dependencies = [
"serde",
"serde_json",
"sha2 0.10.9",
"shlex",
"strum 0.28.0",
"tempfile",
"toml 0.8.23",
@ -3178,6 +3178,7 @@ dependencies = [
"rand 0.9.4",
"serde",
"serde_json",
"shlex",
"tempfile",
"termimad",
"tokio",
@ -3284,7 +3285,6 @@ dependencies = [
"serde",
"serde_json",
"sha2 0.10.9",
"shlex",
"tempfile",
"thiserror 2.0.18",
"tokio",

51
docs/public/api-reference/fabro-api.yaml
View file

@ -13851,16 +13851,61 @@ components:
RunPrepareSettings:
type: object
required: [commands, timeout_ms]
required: [steps, timeout_ms]
properties:
commands:
steps:
type: array
items:
type: string
$ref: "#/components/schemas/PreparedStep"
timeout_ms:
type: integer
format: int64
PreparedStep:
description: |
A single resolved prepare step. The runnable part preserves the
script-vs-argv distinction via the `type` discriminator: a `script`
is a raw shell snippet kept verbatim, while a `command` is an argv
whose elements are shell-quoted and joined at the run boundary (after
`{{ env.* }}` resolution) so an interpolated value cannot inject shell
syntax. Optional per-step `env` is shared by both shapes.
type: object
required: [type]
oneOf:
- $ref: "#/components/schemas/PreparedScriptStep"
- $ref: "#/components/schemas/PreparedCommandStep"
discriminator:
propertyName: type
mapping:
script: "#/components/schemas/PreparedScriptStep"
command: "#/components/schemas/PreparedCommandStep"
PreparedScriptStep:
type: object
required: [type, script]
properties:
type:
type: string
enum: [script]
script:
type: string
env:
$ref: "#/components/schemas/StringMap"
PreparedCommandStep:
type: object
required: [type, command]
properties:
type:
type: string
enum: [command]
command:
type: array
items:
type: string
env:
$ref: "#/components/schemas/StringMap"
RunExecutionSettings:
type: object
required: [mode, approval]

3
lib/crates/fabro-acp/Cargo.toml
View file

@ -11,7 +11,6 @@ default = ["runtime"]
runtime = [
"dep:fabro-sandbox",
"dep:fabro-types",
"dep:fabro-util",
"dep:futures",
"dep:tokio",
"dep:tokio-util",
@ -30,7 +29,7 @@ agent-client-protocol.workspace = true
agent-client-protocol-tokio.workspace = true
fabro-sandbox = { path = "../fabro-sandbox", optional = true }
fabro-types = { path = "../fabro-types", optional = true }
fabro-util = { path = "../fabro-util", optional = true }
fabro-util = { path = "../fabro-util" }
serde_json.workspace = true
shlex = "1"
thiserror.workspace = true

14
lib/crates/fabro-acp/src/command.rs
View file

@ -3,6 +3,7 @@ use std::path::{Path, PathBuf};
use agent_client_protocol::schema::{McpServer, McpServerStdio};
use agent_client_protocol_tokio::AcpAgent;
use fabro_util::shell::shell_join;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AcpProcessSpec {
@ -143,11 +144,7 @@ pub enum AcpCommandError {
}
fn render_command(program: &Path, args: &[String]) -> String {
std::iter::once(program.to_string_lossy().into_owned())
.chain(args.iter().cloned())
.map(|part| shell_quote(&part))
.collect::<Vec<_>>()
.join(" ")
shell_join(std::iter::once(program.to_string_lossy().into_owned()).chain(args.iter().cloned()))
}
fn parse_config_server(raw: &str) -> Result<McpServer, AcpCommandError> {
@ -171,13 +168,6 @@ fn parse_config_server(raw: &str) -> Result<McpServer, AcpCommandError> {
serde_json::from_value(value).map_err(AcpCommandError::InvalidConfigJson)
}
fn shell_quote(s: &str) -> String {
shlex::try_quote(s).map_or_else(
|_| format!("'{}'", s.replace('\'', "'\\''")),
|quoted| quoted.to_string(),
)
}
#[cfg(test)]
mod tests {
use std::path::Path;

2
lib/crates/fabro-cli/tests/it/cmd/attach.rs
View file

@ -995,7 +995,7 @@ fn attach_json_errors_without_prompting_for_human_input() {
},
"notifications": {},
"prepare": {
"commands": [],
"steps": [],
"timeout_ms": 300000
},
"pull_request": null,

8
lib/crates/fabro-cli/tests/it/cmd/config.rs
View file

@ -357,10 +357,12 @@ fn create_explicit_workflow_path_uses_project_config_relative_to_workflow() {
run_spec["settings"]["run"]["model"]["name"].as_str(),
Some("gpt-5.4-pro")
);
// v2 R30: run.prepare.steps replaces the whole ordered list across layers.
// run.prepare.steps replaces the whole ordered list across layers. A
// `script` step serializes with the `type` discriminator that preserves the
// script-vs-argv distinction in the run spec wire shape.
assert_eq!(
run_spec["settings"]["run"]["prepare"]["commands"],
serde_json::json!(["workflow-setup"])
run_spec["settings"]["run"]["prepare"]["steps"],
serde_json::json!([{ "type": "script", "script": "workflow-setup" }])
);
}

2
lib/crates/fabro-cli/tests/it/cmd/inspect.rs
View file

@ -139,7 +139,7 @@ fn inspect_resolves_selector_via_server_endpoint() {
"author": null
},
"prepare": {
"commands": [],
"steps": [],
"timeout_ms": 300000
},
"execution": {

8
lib/crates/fabro-config/src/resolve/mod.rs
View file

@ -73,10 +73,10 @@ pub(crate) fn default_string(path: impl AsRef<std::path::Path>) -> String {
path.as_ref().to_string_lossy().into_owned()
}
/// Warn when a field demoted out of the interpolation set (D2) still contains
/// claimed template tokens. These fields are plain `String` now — `{{ vars.*
/// }}` (which previously substituted via the run-scoped String pass, a
/// now-removed accident) and `{{ env.* }}` are both treated as literal text.
/// Warn when a field demoted out of the interpolation set still contains
/// claimed template tokens. These fields are plain `String` now, so
/// `{{ vars.* }}` (which previously substituted via the run-scoped String pass,
/// a now-removed accident) and `{{ env.* }}` are both treated as literal text.
/// Other plain-`String` fields still substitute `{{ vars.* }}` until the
/// String pass itself is retired in a later slice. Unclaimed `{{ ... }}` text
/// (jq programs, Go templates) never interpolated and does not warn.

159
lib/crates/fabro-config/src/resolve/run.rs
View file

@ -4,11 +4,11 @@ use fabro_types::settings::InterpString;
use fabro_types::settings::run::{
ArtifactsSettings, GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings,
McpServerSettings, McpTransport, MergeStrategy, NotificationProviderSettings,
NotificationRouteSettings, PullRequestSettings, ResolvedMcpEntry, RunAgentSettings,
RunBranchSettings, RunCheckpointSettings, RunCloneSettings, RunExecutionSettings,
RunGitSettings, RunGoal, RunIntegrationsGithubSettings, RunIntegrationsSettings,
RunInterviewsSettings, RunMetaBranchSettings, RunModelControls, RunModelSettings, RunNamespace,
RunPrepareSettings, RunScmSettings, ScmGitHubSettings, TlsMode,
NotificationRouteSettings, PreparedStep, PreparedStepRun, PullRequestSettings,
ResolvedMcpEntry, RunAgentSettings, RunBranchSettings, RunCheckpointSettings, RunCloneSettings,
RunExecutionSettings, RunGitSettings, RunGoal, RunIntegrationsGithubSettings,
RunIntegrationsSettings, RunInterviewsSettings, RunMetaBranchSettings, RunModelControls,
RunModelSettings, RunNamespace, RunPrepareSettings, RunScmSettings, ScmGitHubSettings, TlsMode,
};
use super::{ResolveError, resolve_run_environment};
@ -154,8 +154,9 @@ fn resolve_git(git: Option<&RunGitLayer>) -> RunGitSettings {
#[expect(
clippy::disallowed_methods,
reason = "known leak: prepare step templates collapse to raw source unresolved; strict \
resolution scheduled for follow-up interpolation cleanup"
reason = "intentional source preservation: prepare step commands and per-step env are carried \
in source form so `fabro validate` stays portable; their {{ env.* }} tokens resolve \
at the run boundary in fabro_types::settings::run::RunPrepareSettings::resolve_step_env"
)]
fn resolve_prepare(
prepare: Option<&RunPrepareLayer>,
@ -163,25 +164,44 @@ fn resolve_prepare(
) -> RunPrepareSettings {
let prepare = prepare.expect("defaults.toml should provide run.prepare defaults");
let mut commands = Vec::new();
let mut steps = Vec::new();
for (index, step) in prepare.steps.iter().enumerate() {
match (&step.script, &step.command) {
(Some(script), None) => commands.push(script.as_source()),
(None, Some(argv)) => commands.push(
argv.iter()
.map(InterpString::as_source)
.collect::<Vec<_>>()
.join(" "),
),
(Some(_), Some(_)) | (None, None) => errors.push(ResolveError::Invalid {
path: format!("run.prepare.steps[{index}]"),
reason: "exactly one of script or command must be set".to_string(),
}),
}
let run = match (&step.script, &step.command) {
// A `script` is a raw shell snippet: carry it verbatim so the shell
// interprets it. Its `{{ env.* }}` tokens resolve at the run
// boundary.
(Some(script), None) => PreparedStepRun::Script {
script: script.as_source(),
},
// A `command` is an argv: carry it as a vector of element source
// strings — neither pre-joined nor shell-quoted here. Each element's
// `{{ env.* }}` token resolves at the run boundary, and only the
// resolved value is shell-quoted (resolve-then-quote), so an
// interpolated value can never break out of its argument and inject
// shell syntax.
(None, Some(argv)) => PreparedStepRun::Command {
command: argv.iter().map(InterpString::as_source).collect(),
},
(Some(_), Some(_)) | (None, None) => {
errors.push(ResolveError::Invalid {
path: format!("run.prepare.steps[{index}]"),
reason: "exactly one of script or command must be set".to_string(),
});
continue;
}
};
steps.push(PreparedStep {
run,
env: step
.env
.iter()
.map(|(key, value)| (key.clone(), value.as_source()))
.collect(),
});
}
RunPrepareSettings {
commands,
steps,
timeout_ms: prepare.timeout.map_or(300_000, |timeout| {
u64::try_from(timeout.as_std().as_millis()).unwrap_or(u64::MAX)
}),
@ -603,3 +623,98 @@ fn resolve_artifacts(artifacts: Option<&RunArtifactsLayer>) -> ArtifactsSettings
.unwrap_or_default(),
}
}
#[cfg(test)]
mod resolve_prepare_tests {
use std::collections::HashMap;
use fabro_types::settings::InterpString;
use fabro_types::settings::run::PreparedStepRun;
use super::{ResolveError, resolve_prepare};
use crate::{PrepareStep, RunPrepareLayer};
fn resolve(layer: &RunPrepareLayer) -> Vec<super::PreparedStep> {
let mut errors: Vec<ResolveError> = Vec::new();
let settings = resolve_prepare(Some(layer), &mut errors);
assert!(errors.is_empty(), "unexpected resolve errors: {errors:?}");
settings.steps
}
#[test]
fn carries_per_step_env_through() {
let layer = RunPrepareLayer {
steps: vec![PrepareStep {
script: Some(InterpString::parse("setup")),
command: None,
env: HashMap::from([(
"TOKEN".to_string(),
InterpString::parse("{{ env.DEPLOY_TOKEN }}"),
)]),
}],
timeout: None,
};
let steps = resolve(&layer);
assert_eq!(steps.len(), 1);
assert_eq!(steps[0].run, PreparedStepRun::Script {
script: "setup".to_string(),
});
// The per-step env survives resolution in source form (its {{ env.* }}
// token resolves later, at the run boundary).
assert_eq!(
steps[0].env.get("TOKEN").map(String::as_str),
Some("{{ env.DEPLOY_TOKEN }}")
);
}
#[test]
fn argv_command_elements_are_carried_in_source_form() {
let layer = RunPrepareLayer {
steps: vec![PrepareStep {
script: None,
command: Some(vec![
InterpString::parse("echo"),
InterpString::parse("hello world"),
InterpString::parse("{{ env.USER_INPUT }}"),
]),
env: HashMap::new(),
}],
timeout: None,
};
let steps = resolve(&layer);
// Argv elements are carried as separate source strings, NOT joined and
// NOT shell-quoted here. Quoting happens at the run boundary, after
// `{{ env.* }}` resolution, so the resolved value (not the source
// token) is what gets quoted.
assert_eq!(steps[0].run, PreparedStepRun::Command {
command: vec![
"echo".to_string(),
"hello world".to_string(),
"{{ env.USER_INPUT }}".to_string(),
],
});
}
#[test]
fn script_is_kept_verbatim() {
let layer = RunPrepareLayer {
steps: vec![PrepareStep {
script: Some(InterpString::parse("echo hello && ls -la")),
command: None,
env: HashMap::new(),
}],
timeout: None,
};
let steps = resolve(&layer);
// A script is a raw shell snippet, not an argv: it is carried verbatim.
assert_eq!(steps[0].run, PreparedStepRun::Script {
script: "echo hello && ls -la".to_string(),
});
}
}

2
lib/crates/fabro-dev/Cargo.toml
View file

@ -32,9 +32,9 @@ chrono.workspace = true
clap.workspace = true
csv = "1"
fabro-config = { path = "../fabro-config" }
fabro-util = { path = "../fabro-util" }
fabro-options-metadata.workspace = true
quick-xml = "0.36"
shlex = "1"
toml_edit.workspace = true
tracing-subscriber.workspace = true
walkdir.workspace = true

7
lib/crates/fabro-dev/src/commands/mod.rs
View file

@ -17,6 +17,7 @@ pub(crate) use bench_tests::{BenchTestsArgs, bench_tests};
pub(crate) use build::{BuildArgs, build};
pub(crate) use docker_build::{DockerBuildArgs, docker_build};
pub(crate) use docs::{DocsArgs, docs};
use fabro_util::shell::shell_quote;
pub(crate) use release::{ReleaseArgs, release};
pub(crate) use spa::{SpaArgs, spa};
@ -188,11 +189,7 @@ fn is_cargo_build_env(key: &std::ffi::OsStr) -> bool {
}
pub(crate) fn shell_arg(arg: impl AsRef<str>) -> String {
let arg = arg.as_ref();
shlex::try_quote(arg).map_or_else(
|_| format!("'{}'", arg.replace('\'', "'\\''")),
std::borrow::Cow::into_owned,
)
shell_quote(arg.as_ref())
}
#[cfg(test)]

1
lib/crates/fabro-sandbox/Cargo.toml
View file

@ -38,7 +38,6 @@ fabro-proc = { path = "../fabro-proc" }
fabro-static.workspace = true
fabro-util = { path = "../fabro-util" }
fabro-redact.workspace = true
shlex = "1"
# local
glob = { version = "0.3" }

9
lib/crates/fabro-sandbox/src/sandbox.rs
View file

@ -8,6 +8,7 @@ use std::time::Duration;
use async_trait::async_trait;
use fabro_types::{CommandOutputStream, CommandTermination};
use fabro_util::shell;
use serde::{Deserialize, Serialize};
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite};
use tokio::sync::Mutex as TokioMutex;
@ -1049,12 +1050,10 @@ pub(crate) fn resolve_path(path: &str, working_dir: &str) -> String {
}
/// Shell-quote a string using `shlex::try_quote`, with a fallback for edge
/// cases.
/// cases. Re-exported from [`fabro_util::shell::shell_quote`] so sandbox code
/// and the config resolve layer share one audited implementation.
pub fn shell_quote(s: &str) -> String {
shlex::try_quote(s).map_or_else(
|_| format!("'{}'", s.replace('\'', "'\\''")),
|q| q.to_string(),
)
shell::shell_quote(s)
}
/// Helper for sandbox implementations that manage git internally.

19
lib/crates/fabro-server/src/demo/mod.rs
View file

@ -1088,8 +1088,8 @@ mod runs {
use fabro_api::types::*;
use fabro_types::settings::run::{
EnvironmentImageSettings, EnvironmentLifecycleSettings, EnvironmentProvider,
EnvironmentResourcesSettings, EnvironmentSettings, RunEnvironmentSettings, RunGoal,
RunModelSettings, RunNamespace, RunPrepareSettings,
EnvironmentResourcesSettings, EnvironmentSettings, PreparedStep, PreparedStepRun,
RunEnvironmentSettings, RunGoal, RunModelSettings, RunNamespace, RunPrepareSettings,
};
use fabro_types::settings::{InterpString, ProjectNamespace, WorkflowNamespace};
use fabro_types::{
@ -1802,7 +1802,20 @@ mod runs {
..RunModelSettings::default()
},
prepare: RunPrepareSettings {
commands: vec!["bun install".into(), "bun run typecheck".into()],
steps: vec![
PreparedStep {
run: PreparedStepRun::Script {
script: "bun install".to_string(),
},
env: HashMap::new(),
},
PreparedStep {
run: PreparedStepRun::Script {
script: "bun run typecheck".to_string(),
},
env: HashMap::new(),
},
],
timeout_ms: 120_000,
},
environment: RunEnvironmentSettings::from_environment(

14
lib/crates/fabro-server/src/run_manifest.rs
View file

@ -566,7 +566,7 @@ async fn build_preflight_report(
}
fn base_preflight_checks(prepared: &PreparedManifest, graph: &Graph) -> Vec<CheckResult> {
let setup_command_count = prepared.settings.run.prepare.commands.len();
let setup_command_count = prepared.settings.run.prepare.steps.len();
let repo_summary = prepared.git.as_ref().map_or_else(
|| "unknown".to_string(),
|git| {
@ -1940,11 +1940,13 @@ app_id = "fixture-app-id"
let prepared = prepare_manifest(&server_settings, &manifest).unwrap();
let settings_json = serde_json::to_value(&prepared.settings).unwrap();
// v2 merge matrix: run.prepare.steps replaces the whole list across
// layers, so the higher-precedence workflow layer wins over cli.
assert_eq!(prepared.settings.run.prepare.commands, vec![
"workflow-setup".to_string()
]);
// run.prepare.steps replaces the whole list across layers, so the
// higher-precedence workflow layer wins over cli.
assert_eq!(prepared.settings.run.prepare.steps.len(), 1);
assert_eq!(
prepared.settings.run.prepare.steps[0].to_shell_command(),
"workflow-setup"
);
assert!(settings_json.pointer("/server").is_none());
}

1
lib/crates/fabro-types/Cargo.toml
View file

@ -34,4 +34,5 @@ url.workspace = true
[dev-dependencies]
fabro-types = { path = ".", features = ["test-support"] }
shlex = "1"
tempfile = "3"

10
lib/crates/fabro-types/src/settings/mod.rs
View file

@ -38,11 +38,11 @@ pub use run::{
EnvironmentNetworkMode, EnvironmentNetworkSettings, EnvironmentProvider,
EnvironmentResourcesSettings, EnvironmentSettings, GitAuthorSettings, HookDefinition, HookType,
InterviewProviderSettings, McpServerRef, McpServerSettings, McpTransport,
NotificationProviderSettings, NotificationRouteSettings, PullRequestSettings, ResolvedMcpEntry,
RunAgentSettings, RunCheckpointSettings, RunEnvironmentSettings, RunExecutionSettings,
RunGitSettings, RunGoal, RunIntegrationsGithubSettings, RunIntegrationsSettings,
RunInterviewsSettings, RunModelControls, RunModelSettings, RunNamespace, RunPrepareSettings,
RunScmSettings, ScmGitHubSettings, TlsMode,
NotificationProviderSettings, NotificationRouteSettings, PreparedStep, PullRequestSettings,
ResolvedMcpEntry, RunAgentSettings, RunCheckpointSettings, RunEnvironmentSettings,
RunExecutionSettings, RunGitSettings, RunGoal, RunIntegrationsGithubSettings,
RunIntegrationsSettings, RunInterviewsSettings, RunModelControls, RunModelSettings,
RunNamespace, RunPrepareSettings, RunScmSettings, ScmGitHubSettings, TlsMode,
};
pub use server::{
GithubIntegrationSettings, IntegrationWebhooksSettings, LogDestination, ObjectStoreSettings,

359
lib/crates/fabro-types/src/settings/run.rs
View file

@ -10,6 +10,7 @@ use std::collections::HashMap;
use std::path::PathBuf;
use std::time::Duration as StdDuration;
use fabro_util::shell;
use serde::de::{self, Deserializer};
use serde::ser::SerializeStruct;
use serde::{Deserialize, Serialize};
@ -104,9 +105,10 @@ impl RunNamespace {
substitute_option(&mut slack.channel, &mut lookup)?;
}
substitute_map(&mut self.integrations.github.permissions, &mut lookup)?;
// run.scm.owner/repository were demoted and removed from this pass
// (D2): values stay literal.
substitute_string_vec(&mut self.prepare.commands, &mut lookup)?;
// run.scm.owner/repository are plain strings: values stay literal.
for step in &mut self.prepare.steps {
visit_prepared_step_strings(step, &mut |value| substitute_string(value, &mut lookup))?;
}
// Only resolved inline servers carry substitutable templates; an
// unresolved reference holds just an id + enabled flag.
for entry in self.agent.mcps.values_mut() {
@ -262,6 +264,28 @@ where
}
}
/// Walk every interpolatable string in one prepare step — the runnable part
/// (a `script` snippet or each `command` argv element) and every per-step `env`
/// value. Both interpolation passes route through this one traversal so they
/// cannot drift as `PreparedStepRun` or `PreparedStep` grow fields: the
/// `{{ vars.* }}` pass ([`RunNamespace::substitute_variables`]) passes a
/// `substitute_string` visitor, the `{{ env.* }}` pass
/// ([`RunPrepareSettings::resolve_step_env`]) passes a `resolve_env_string`
/// visitor. Mirrors [`visit_mcp_transport_strings`].
fn visit_prepared_step_strings<F>(
step: &mut PreparedStep,
visitor: &mut F,
) -> Result<(), ResolveError>
where
F: FnMut(&mut String) -> Result<(), ResolveError>,
{
match &mut step.run {
PreparedStepRun::Script { script } => visitor(script)?,
PreparedStepRun::Command { command } => visit_string_vec(command, visitor)?,
}
visit_string_map(&mut step.env, visitor)
}
fn visit_string_vec<F>(values: &mut [String], visitor: &mut F) -> Result<(), ResolveError>
where
F: FnMut(&mut String) -> Result<(), ResolveError>,
@ -342,7 +366,7 @@ mod run_namespace_variable_substitution_tests {
use super::{
ArtifactsSettings, DockerfileSource, EnvironmentImageSettings, EnvironmentNetworkMode,
EnvironmentNetworkSettings, HookDefinition, HookEvent, HookType, InterpString,
McpHttpProtocol, McpServerSettings, McpTransport, RunCheckpointSettings,
McpHttpProtocol, McpServerSettings, McpTransport, PreparedStepRun, RunCheckpointSettings,
RunEnvironmentSettings, RunGoal, RunNamespace, RunPrepareSettings,
};
@ -357,7 +381,19 @@ mod run_namespace_variable_substitution_tests {
"deploy {{ vars.ENV }} in {{ env.REGION }}",
))),
prepare: RunPrepareSettings {
commands: vec!["echo {{ vars.ENV }} {{ env.REGION }}".to_string()],
steps: vec![super::PreparedStep {
run: PreparedStepRun::Command {
command: vec![
"echo".to_string(),
"{{ vars.ENV }}".to_string(),
"{{ env.REGION }}".to_string(),
],
},
env: HashMap::from([(
"STAGE".to_string(),
"{{ vars.ENV }}-{{ env.REGION }}".to_string(),
)]),
}],
timeout_ms: 1_000,
},
agent: super::RunAgentSettings {
@ -417,9 +453,21 @@ mod run_namespace_variable_substitution_tests {
goal_source,
Some("deploy prod in {{ env.REGION }}".to_string())
);
assert_eq!(run.prepare.commands, vec![
"echo prod {{ env.REGION }}".to_string()
assert_eq!(run.prepare.steps.len(), 1);
// `{{ vars.* }}` substitutes per argv element while `{{ env.* }}` is
// left for the run boundary.
let PreparedStepRun::Command { command } = &run.prepare.steps[0].run else {
panic!("expected command argv prepare step");
};
assert_eq!(command.as_slice(), [
"echo".to_string(),
"prod".to_string(),
"{{ env.REGION }}".to_string(),
]);
assert_eq!(
run.prepare.steps[0].env.get("STAGE").map(String::as_str),
Some("prod-{{ env.REGION }}")
);
let mcp = run.agent.mcps["http"]
.as_resolved()
.expect("expected resolved inline mcp entry");
@ -671,19 +719,105 @@ pub struct GitAuthorSettings {
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct RunPrepareSettings {
pub commands: Vec<String>,
pub steps: Vec<PreparedStep>,
pub timeout_ms: u64,
}
impl Default for RunPrepareSettings {
fn default() -> Self {
Self {
commands: Vec::new(),
steps: Vec::new(),
timeout_ms: 300_000,
}
}
}
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
/// [`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`).
/// 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
/// [`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>,
) -> 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)
})?;
}
Ok(resolved)
}
}
/// A single resolved prepare step: the thing to run plus the per-step
/// environment variables it should see. The runnable part keeps the
/// script-vs-argv distinction (see [`PreparedStepRun`]), and every string is
/// carried in source form out of the config resolve layer; their `{{ env.* }}`
/// tokens resolve at the run boundary via
/// [`RunPrepareSettings::resolve_step_env`].
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct PreparedStep {
#[serde(flatten)]
pub run: PreparedStepRun,
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub env: HashMap<String, String>,
}
/// The runnable part of a prepare step, preserving the script-vs-argv
/// distinction so each is treated correctly when assembled into the shell
/// command that runs via `bash -c`:
///
/// - [`Script`](PreparedStepRun::Script) is a raw shell snippet kept verbatim
/// for the shell to interpret.
/// - [`Command`](PreparedStepRun::Command) is an argv: a vector of element
/// source strings, neither pre-joined nor shell-quoted at config time. Its
/// `{{ env.* }}` tokens resolve per element at the run boundary, and only
/// then is each *resolved* element shell-quoted and joined. Resolving before
/// quoting is what stops an interpolated env value from breaking out of its
/// argument and injecting shell syntax.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum PreparedStepRun {
Script { script: String },
Command { command: Vec<String> },
}
impl PreparedStep {
/// Flatten this step's runnable part into the single shell string that runs
/// via `bash -c`.
///
/// For a script, the snippet is returned verbatim. For an argv `command`,
/// each element is shell-quoted and joined with spaces so an argument that
/// contains spaces or shell metacharacters survives as a single token. This
/// must run *after* [`RunPrepareSettings::resolve_step_env`] so the quoting
/// applies to the resolved values, not the `{{ env.* }}` source.
pub fn to_shell_command(&self) -> String {
match &self.run {
PreparedStepRun::Script { script } => script.clone(),
PreparedStepRun::Command { command } => shell::shell_join(command),
}
}
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct RunExecutionSettings {
pub mode: RunMode,
@ -1556,6 +1690,213 @@ mod resolve_transport_env_tests {
}
}
#[cfg(test)]
mod resolve_step_env_tests {
use std::collections::HashMap;
use super::super::interp::ResolveErrorKind;
use super::{Namespace, PreparedStep, PreparedStepRun, RunPrepareSettings};
fn env_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 {
script: script.to_string(),
},
env,
}
}
fn command_step(argv: &[&str], env: HashMap<String, String>) -> PreparedStep {
PreparedStep {
run: PreparedStepRun::Command {
command: argv.iter().map(|element| (*element).to_string()).collect(),
},
env,
}
}
#[test]
fn literal_step_passes_through() {
let settings = RunPrepareSettings {
steps: vec![script_step(
"echo hello",
HashMap::from([("STAGE".to_string(), "build".to_string())]),
)],
timeout_ms: 1_000,
};
let resolved = settings.resolve_step_env(env_lookup(&[])).unwrap();
assert_eq!(resolved.steps[0].to_shell_command(), "echo hello");
assert_eq!(
resolved.steps[0].env.get("STAGE").map(String::as_str),
Some("build")
);
}
#[test]
fn script_resolves_verbatim() {
// A script is a raw shell snippet: its `{{ env.* }}` token resolves but
// the result is NOT shell-quoted — the shell interprets the snippet as
// written.
let settings = RunPrepareSettings {
steps: vec![script_step(
"deploy {{ env.REGION }} && echo done",
HashMap::new(),
)],
timeout_ms: 1_000,
};
let resolved = settings
.resolve_step_env(env_lookup(&[("REGION", "us-east-1")]))
.unwrap();
assert_eq!(
resolved.steps[0].to_shell_command(),
"deploy us-east-1 && echo done"
);
}
#[test]
fn command_and_env_resolve() {
let settings = RunPrepareSettings {
steps: vec![command_step(
&["deploy", "{{ env.REGION }}"],
HashMap::from([("TOKEN".to_string(), "{{ env.DEPLOY_TOKEN }}".to_string())]),
)],
timeout_ms: 1_000,
};
let resolved = settings
.resolve_step_env(env_lookup(&[
("REGION", "us-east-1"),
("DEPLOY_TOKEN", "secret-token"),
]))
.unwrap();
assert_eq!(resolved.steps[0].to_shell_command(), "deploy us-east-1");
assert_eq!(
resolved.steps[0].env.get("TOKEN").map(String::as_str),
Some("secret-token")
);
}
#[test]
fn command_arg_with_space_stays_one_token() {
// A resolved argv element that contains a space must survive as a
// single shell word, not re-split into two.
let settings = RunPrepareSettings {
steps: vec![command_step(&["echo", "{{ env.MESSAGE }}"], HashMap::new())],
timeout_ms: 1_000,
};
let resolved = settings
.resolve_step_env(env_lookup(&[("MESSAGE", "hello world")]))
.unwrap();
let shell = resolved.steps[0].to_shell_command();
let tokens = shlex::split(&shell).expect("resolved command should be valid shell");
assert_eq!(tokens, vec!["echo".to_string(), "hello world".to_string()]);
}
#[test]
fn command_arg_resolving_to_shell_metacharacters_is_not_injected() {
// Regression test for the command-injection defect: an `{{ env.* }}`
// value containing a single quote and `;` must be resolved THEN quoted
// so it stays a single argument and cannot break out to inject extra
// shell commands. Quoting the source token *before* resolving (the old
// behavior) lets the substituted value escape its quotes.
let malicious = "x'; touch PWNED; echo '";
let settings = RunPrepareSettings {
steps: vec![command_step(
&["echo", "{{ env.USER_INPUT }}"],
HashMap::new(),
)],
timeout_ms: 1_000,
};
let resolved = settings
.resolve_step_env(|name| (name == "USER_INPUT").then(|| malicious.to_string()))
.unwrap();
let shell = resolved.steps[0].to_shell_command();
// The flattened shell string round-trips to EXACTLY two tokens: the
// command and the verbatim payload as a single argument. Pre-fix, the
// value was substituted raw inside config-time quotes
// (`echo 'x'; touch PWNED; echo ''`), which `shlex::split` parses as
// several tokens / an injected `touch PWNED` command — so the round-trip
// equality below fails on the buggy code and passes once the value is
// resolved THEN quoted.
let tokens = shlex::split(&shell).expect("resolved command should be valid shell");
assert_eq!(tokens, vec!["echo".to_string(), malicious.to_string()]);
assert_eq!(
tokens.len(),
2,
"injected shell syntax leaked extra tokens: {shell}"
);
}
#[test]
fn missing_env_in_command_is_hard_error() {
let settings = RunPrepareSettings {
steps: vec![command_step(
&["deploy", "{{ env.REGION }}"],
HashMap::new(),
)],
timeout_ms: 1_000,
};
let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err();
assert_eq!(err.namespace, Namespace::Env);
assert_eq!(err.name, "REGION");
assert_eq!(err.kind, ResolveErrorKind::Missing);
}
#[test]
fn missing_env_in_step_env_value_is_hard_error() {
let settings = RunPrepareSettings {
steps: vec![script_step(
"echo hi",
HashMap::from([("TOKEN".to_string(), "{{ env.DEPLOY_TOKEN }}".to_string())]),
)],
timeout_ms: 1_000,
};
let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err();
assert_eq!(err.namespace, Namespace::Env);
assert_eq!(err.name, "DEPLOY_TOKEN");
assert_eq!(err.kind, ResolveErrorKind::Missing);
}
#[test]
fn reserved_secret_token_is_unavailable_not_leaked() {
let settings = RunPrepareSettings {
steps: vec![script_step(
"echo hi",
HashMap::from([("API_KEY".to_string(), "{{ secrets.API_KEY }}".to_string())]),
)],
timeout_ms: 1_000,
};
let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err();
assert_eq!(err.namespace, Namespace::Secrets);
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
}
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)]
pub enum McpTransport {

1
lib/crates/fabro-util/Cargo.toml
View file

@ -16,6 +16,7 @@ workspace = true
console.workspace = true
fabro-static.workspace = true
rand.workspace = true
shlex = "1"
termimad.workspace = true
serde_json.workspace = true
serde.workspace = true

1
lib/crates/fabro-util/src/lib.rs
View file

@ -11,6 +11,7 @@ pub mod path;
pub mod printer;
pub mod run_log;
pub mod session_secret;
pub mod shell;
pub mod terminal;
pub mod text;
pub mod time;

65
lib/crates/fabro-util/src/shell.rs Normal file
View file

@ -0,0 +1,65 @@
//! Shell-quoting helpers.
//!
//! A single audited place that turns an arbitrary string into a token that is
//! safe to interpolate into a `/bin/sh` command line. Use this anywhere a
//! user-controlled value (path, branch name, URL, env var, glob, argv element)
//! is assembled into a shell script — never hand-roll the escaping.
/// Shell-quote a string using `shlex::try_quote`, with a fallback for edge
/// cases (`shlex` rejects strings containing a NUL byte, which can never appear
/// in a real argv anyway, so the fallback simply single-quotes defensively).
#[must_use]
pub fn shell_quote(s: &str) -> String {
shlex::try_quote(s).map_or_else(
|_| format!("'{}'", s.replace('\'', "'\\''")),
std::borrow::Cow::into_owned,
)
}
/// Shell-quote each element of an argv and join them with spaces into a single
/// `/bin/sh` command line. Quoting is applied per element via [`shell_quote`],
/// so an argument containing spaces or shell metacharacters survives as one
/// shell word instead of being re-split. Use this for any `program + args`
/// vector that is assembled into a shell script.
#[must_use]
pub fn shell_join(parts: impl IntoIterator<Item = impl AsRef<str>>) -> String {
parts
.into_iter()
.map(|part| shell_quote(part.as_ref()))
.collect::<Vec<_>>()
.join(" ")
}
#[cfg(test)]
mod tests {
use super::{shell_join, shell_quote};
#[test]
fn plain_word_is_unquoted() {
assert_eq!(shell_quote("setup"), "setup");
}
#[test]
fn spaces_are_quoted() {
assert_eq!(shell_quote("hello world"), "'hello world'");
}
#[test]
fn single_quotes_are_escaped() {
// shlex double-quotes a token that contains a single quote.
assert_eq!(shell_quote("it's"), r#""it's""#);
}
#[test]
fn join_quotes_each_element() {
assert_eq!(
shell_join(["echo", "hello world", "a;b"]),
r"echo 'hello world' 'a;b'"
);
}
#[test]
fn join_empty_is_empty() {
assert_eq!(shell_join(Vec::<String>::new()), "");
}
}

1
lib/crates/fabro-workflow/Cargo.toml
View file

@ -63,7 +63,6 @@ hex.workspace = true
sha2 = { workspace = true }
mime_guess.workspace = true
miette.workspace = true
shlex = "1"
git2.workspace = true
tokio-util.workspace = true
tracing.workspace = true

9
lib/crates/fabro-workflow/src/handler/command.rs
View file

@ -4,6 +4,7 @@ use async_trait::async_trait;
use fabro_agent::CommandOutputCallback;
use fabro_graphviz::graph::{Graph, Node};
use fabro_types::{CommandTermination, StageTiming};
use fabro_util::shell::shell_quote;
use super::{EngineServices, Handler, NodeTimeoutPolicy};
use crate::command_log::CommandLogRecorder;
@ -16,14 +17,6 @@ fn timeout_ms(node: &Node) -> Option<u64> {
node.timeout().map(crate::millis_u64)
}
/// Shell-escape a string using `shlex::try_quote` (POSIX-safe).
fn shell_quote(s: &str) -> String {
shlex::try_quote(s).map_or_else(
|_| format!("'{}'", s.replace('\'', "'\\''")),
|q| q.to_string(),
)
}
/// Executes an external script configured via node attributes.
pub struct CommandHandler;

38
lib/crates/fabro-workflow/src/operations/start.rs
View file

@ -18,7 +18,7 @@ use fabro_static::EnvVars;
use fabro_types::settings::run::{
ApprovalMode, McpServerSettings as ResolvedMcpServerSettings, PullRequestSettings,
ResolvedMcpEntry, RunMode, RunModelSettings as ResolvedRunModelSettings,
RunNamespace as ResolvedRunSettings,
RunNamespace as ResolvedRunSettings, RunPrepareSettings as ResolvedRunPrepareSettings,
};
use fabro_types::settings::{ModelRegistry, ResolvedModelRef};
use fabro_types::{ManifestPath, RunId, RunRunnableSource, SandboxProviderKind};
@ -44,7 +44,7 @@ use crate::pipeline::{
use crate::records::Checkpoint;
use crate::run_control::RunControlState;
use crate::run_metadata::metadata_branch_name;
use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions};
use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions, SetupCommand};
use crate::run_status::{FailureReason, RunStatus};
use crate::runtime_store::RunStoreHandle;
use crate::services::FabroRunToolServices;
@ -471,7 +471,7 @@ impl RunSession {
steering_hub: services.steering_hub,
on_node: services.on_node,
lifecycle: LifecycleOptions {
setup_commands: resolved.prepare.commands.clone(),
setup_commands: runtime_setup_commands(&resolved.prepare)?,
setup_command_timeout_ms: resolved.prepare.timeout_ms,
},
hooks: fabro_hooks::HookSettings {
@ -703,6 +703,38 @@ fn runtime_mcp_server(
})
}
/// 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.
///
/// 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.
fn runtime_setup_commands(
prepare: &ResolvedRunPrepareSettings,
) -> Result<Vec<SetupCommand>, Error> {
let resolved = prepare
.resolve_step_env(process_env_var)
.map_err(|err| Error::engine_with_source("failed to resolve prepare step", err))?;
Ok(resolved
.steps
.into_iter()
.map(|step| SetupCommand {
// Flatten the runnable part into the shell string AFTER env
// resolution: an argv `command` is shell-quoted per resolved
// element here so an interpolated value stays a single token; a
// `script` is kept verbatim.
command: step.to_shell_command(),
env: step.env,
})
.collect())
}
impl RunSession {
/// Shared engine: initialize, execute, finalize, pull_request.
async fn run(

14
lib/crates/fabro-workflow/src/pipeline/execute/tests.rs
View file

@ -33,7 +33,7 @@ use crate::outcome::{Outcome, OutcomeExt, StageOutcome};
use crate::pipeline::initialize;
use crate::pipeline::types::{InitOptions, LlmSpec, Persisted, SandboxEnvSpec};
use crate::records::RunSpec;
use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions};
use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions, SetupCommand};
use crate::test_support::run_graph;
fn local_env() -> Arc<dyn Sandbox> {
@ -175,9 +175,15 @@ fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: RunI
)
}
fn test_lifecycle(setup_commands: Vec<String>) -> LifecycleOptions {
fn test_lifecycle(setup_commands: Vec<&str>) -> LifecycleOptions {
LifecycleOptions {
setup_commands,
setup_commands: setup_commands
.into_iter()
.map(|command| SetupCommand {
command: command.to_string(),
env: std::collections::HashMap::new(),
})
.collect(),
setup_command_timeout_ms: 300_000,
}
}
@ -1202,7 +1208,7 @@ async fn run_with_lifecycle_emits_initialize_and_setup_events() {
local_env(),
&simple_graph(),
test_run_options(dir.path(), "order-test"),
test_lifecycle(vec!["echo ok".to_string()]),
test_lifecycle(vec!["echo ok"]),
)
.await
.unwrap();

54
lib/crates/fabro-workflow/src/pipeline/initialize.rs
View file

@ -525,19 +525,21 @@ pub async fn initialize(
command_count: options.lifecycle.setup_commands.len(),
});
let setup_start = Instant::now();
for (index, command) in options.lifecycle.setup_commands.iter().enumerate() {
for (index, setup) in options.lifecycle.setup_commands.iter().enumerate() {
let command = &setup.command;
options.emitter.emit(&Event::SetupCommandStarted {
command: command.clone(),
index,
});
let cmd_start = Instant::now();
let cancel_token = options.run_options.cancel_token.child_token();
let step_env = (!setup.env.is_empty()).then_some(&setup.env);
let result = sandbox
.exec_command(
command,
options.lifecycle.setup_command_timeout_ms,
None,
None,
step_env,
Some(cancel_token.clone()),
)
.await
@ -665,6 +667,13 @@ mod tests {
fixtures::RUN_1
}
fn setup_cmd(command: &str) -> crate::run_options::SetupCommand {
crate::run_options::SetupCommand {
command: command.to_string(),
env: HashMap::new(),
}
}
fn test_catalog() -> Arc<Catalog> {
Arc::new(Catalog::from_builtin().expect("default catalog should build"))
}
@ -783,6 +792,12 @@ mod tests {
async fn initialize_with_setup_command(
command: &str,
) -> (crate::error::Result<Initialized>, Vec<RunEvent>) {
initialize_with_setup_step(setup_cmd(command)).await
}
async fn initialize_with_setup_step(
setup: crate::run_options::SetupCommand,
) -> (crate::error::Result<Initialized>, Vec<RunEvent>) {
let temp = tempfile::tempdir().unwrap();
let run_dir = temp.path().join("run");
@ -820,7 +835,7 @@ mod tests {
steering_hub: Arc::new(crate::steering_hub::SteeringHub::new(emitter.clone())),
catalog: test_catalog(),
lifecycle: crate::run_options::LifecycleOptions {
setup_commands: vec![command.to_string()],
setup_commands: vec![setup],
setup_command_timeout_ms: 1_000,
},
run_options: test_settings(&run_dir),
@ -1196,7 +1211,7 @@ mod tests {
steering_hub: Arc::new(crate::steering_hub::SteeringHub::new(emitter.clone())),
catalog: test_catalog(),
lifecycle: crate::run_options::LifecycleOptions {
setup_commands: vec!["true".to_string()],
setup_commands: vec![setup_cmd("true")],
setup_command_timeout_ms: 1_000,
},
run_options: test_settings(&run_dir),
@ -1230,6 +1245,35 @@ mod tests {
);
}
#[tokio::test]
async fn initialize_passes_per_step_env_to_setup_command() {
// The command only succeeds when the per-step env var is visible to the
// shell, so a green run proves the env reached `exec_command`.
let setup = crate::run_options::SetupCommand {
command: "test \"$PREPARE_STAGE\" = build".to_string(),
env: HashMap::from([("PREPARE_STAGE".to_string(), "build".to_string())]),
};
let (result, events) = initialize_with_setup_step(setup).await;
assert!(result.is_ok(), "setup with per-step env should succeed");
assert!(
events
.iter()
.any(|event| event.event_name() == "setup.completed")
);
}
#[tokio::test]
async fn initialize_setup_command_without_step_env_does_not_see_it() {
// Negative control: the same command without the per-step env fails,
// confirming the success above is attributable to the per-step env.
let (result, _events) =
initialize_with_setup_command("test \"$PREPARE_STAGE\" = build").await;
assert!(result.is_err(), "setup should fail without per-step env");
}
#[tokio::test]
async fn initialize_setup_failure_preserves_stderr_and_adds_exec_tail() {
let (result, events) =
@ -1310,7 +1354,7 @@ mod tests {
steering_hub: Arc::new(crate::steering_hub::SteeringHub::new(emitter.clone())),
catalog: test_catalog(),
lifecycle: crate::run_options::LifecycleOptions {
setup_commands: vec!["sleep 5".to_string()],
setup_commands: vec![setup_cmd("sleep 5")],
setup_command_timeout_ms: 5_000,
},
run_options,

14
lib/crates/fabro-workflow/src/run_options.rs
View file

@ -74,8 +74,18 @@ impl RunOptions {
/// Options for sandbox lifecycle management within the engine.
pub struct LifecycleOptions {
/// Setup commands to run inside the sandbox after initialization.
pub setup_commands: Vec<String>,
/// Setup commands to run inside the sandbox after initialization, each with
/// its own environment.
pub setup_commands: Vec<SetupCommand>,
/// Timeout in milliseconds for each setup command.
pub setup_command_timeout_ms: u64,
}
/// A single setup (prepare) command and the per-step environment it runs with.
/// Both the command string and the env values are already fully resolved (their
/// `{{ env.* }}` tokens replaced at the run boundary) by the time they reach
/// the sandbox.
pub struct SetupCommand {
pub command: String,
pub env: std::collections::HashMap<String, String>,
}

3
lib/packages/fabro-api-client/src/.openapi-generator/FILES generated
View file

@ -280,6 +280,9 @@ models/preflight-check-result.ts
models/preflight-check-section.ts
models/preflight-response.ts
models/preflight-workflow-summary.ts
models/prepared-command-step.ts
models/prepared-script-step.ts
models/prepared-step.ts
models/preview-url-request.ts
models/preview-url-response.ts
models/principal-agent.ts

3
lib/packages/fabro-api-client/src/models/index.ts generated
View file

@ -250,6 +250,9 @@ export * from './preflight-check-result';
export * from './preflight-check-section';
export * from './preflight-response';
export * from './preflight-workflow-summary';
export * from './prepared-command-step';
export * from './prepared-script-step';
export * from './prepared-step';
export * from './preview-url-request';
export * from './preview-url-response';
export * from './principal';

27
lib/packages/fabro-api-client/src/models/prepared-command-step.ts generated Normal file
View file

@ -0,0 +1,27 @@
/* tslint:disable */
/* eslint-disable */
/**
* Fabro Run API
* HTTP API for managing Fabro workflow run executions.
*
* The version of the OpenAPI document: 0.1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
export interface PreparedCommandStep {
'type': PreparedCommandStepTypeEnum;
'command': Array<string>;
'env'?: { [key: string]: string; };
}
export const PreparedCommandStepTypeEnum = {
COMMAND: 'command'
} as const;
export type PreparedCommandStepTypeEnum = typeof PreparedCommandStepTypeEnum[keyof typeof PreparedCommandStepTypeEnum];

27
lib/packages/fabro-api-client/src/models/prepared-script-step.ts generated Normal file
View file

@ -0,0 +1,27 @@
/* tslint:disable */
/* eslint-disable */
/**
* Fabro Run API
* HTTP API for managing Fabro workflow run executions.
*
* The version of the OpenAPI document: 0.1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
export interface PreparedScriptStep {
'type': PreparedScriptStepTypeEnum;
'script': string;
'env'?: { [key: string]: string; };
}
export const PreparedScriptStepTypeEnum = {
SCRIPT: 'script'
} as const;
export type PreparedScriptStepTypeEnum = typeof PreparedScriptStepTypeEnum[keyof typeof PreparedScriptStepTypeEnum];

27
lib/packages/fabro-api-client/src/models/prepared-step.ts generated Normal file
View file

@ -0,0 +1,27 @@
/* tslint:disable */
/* eslint-disable */
/**
* Fabro Run API
* HTTP API for managing Fabro workflow run executions.
*
* The version of the OpenAPI document: 0.1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
// May contain unused imports in some cases
// @ts-ignore
import type { PreparedCommandStep } from './prepared-command-step';
// May contain unused imports in some cases
// @ts-ignore
import type { PreparedScriptStep } from './prepared-script-step';
/**
* @type PreparedStep
* A single resolved prepare step. The runnable part preserves the script-vs-argv distinction via the `type` discriminator: a `script` is a raw shell snippet kept verbatim, while a `command` is an argv whose elements are shell-quoted and joined at the run boundary (after `{{ env.* }}` resolution) so an interpolated value cannot inject shell syntax. Optional per-step `env` is shared by both shapes.
*/
export type PreparedStep = { type: 'command' } & PreparedCommandStep | { type: 'script' } & PreparedScriptStep;

5
lib/packages/fabro-api-client/src/models/run-prepare-settings.ts generated
View file

@ -13,8 +13,11 @@
*/
// May contain unused imports in some cases
// @ts-ignore
import type { PreparedStep } from './prepared-step';
export interface RunPrepareSettings {
'commands': Array<string>;
'steps': Array<PreparedStep>;
'timeout_ms': number;
}