scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-11 03:40:05 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

feat(hooks): typed end-to-end interpolation with fail-closed resolution

Browse source 
Carry hook command, URL, header, prompt, and model fields as a typed
InterpString from the config resolve layer all the way to the executor,
so hook segments resolve at fire time from the typed value instead of
collapsing to a String and re-parsing it. This mirrors the MCP transport
env resolution boundary.

Replace the MiniJinja-with-allowlist on HTTP-hook headers with the same
narrow `{{ ns.NAME }}` token resolution used everywhere else — no
template engine on headers.

Make token resolution fail-closed and consistent across hook types. A
missing or out-of-scope token in a command, URL, header, prompt, or
model is now a hard error that blocks the hook rather than firing it with
a half-resolved or empty value. Previously command hooks failed closed
but http/prompt/agent hooks failed open (warn and proceed), which could
dispatch an HTTP request with an empty credential header or run an LLM
call against a half-rendered prompt.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-06-25 15:18:20 -04:00
parent 7507a2279a
commit 24526304b9
8 changed files with 279 additions and 181 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -101,6 +101,7 @@ pub(crate) fn warn_if_demoted_template(field: &str, value: Option<&str>) {
mod tests {
use std::collections::HashMap;
use fabro_types::settings::InterpString;
use fabro_types::settings::run::{
HookType, McpHttpProtocol, McpTransport, ResolvedMcpEntry, TlsMode,
};
@ -201,10 +202,10 @@ Authorization = "Bearer {{ env.HOOK_TOKEN }}"
assert_eq!(
hook.resolved_hook_type().as_deref(),
Some(&HookType::Http {
url: "https://hooks.example.com".to_string(),
url: InterpString::parse("https://hooks.example.com"),
headers: Some(HashMap::from([(
"Authorization".to_string(),
"Bearer {{ env.HOOK_TOKEN }}".to_string(),
InterpString::parse("Bearer {{ env.HOOK_TOKEN }}"),
)])),
allowed_env_vars: Vec::new(),
tls: TlsMode::Verify,

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

@ -412,11 +412,6 @@ fn resolve_mcp_command(
.unwrap_or_default()
}
#[expect(
clippy::disallowed_methods,
reason = "intentional source preservation: the hook executor re-resolves {{ env.* }} \
tokens at hook fire time"
)]
fn resolve_hook(hook: &HookEntry, index: usize, errors: &mut Vec<ResolveError>) -> HookDefinition {
let variants = [
hook.script.is_some() || hook.command.is_some(),
@ -437,15 +432,9 @@ fn resolve_hook(hook: &HookEntry, index: usize, errors: &mut Vec<ResolveError>)
let hook_type = resolve_hook_type(hook);
let command = if let Some(script) = &hook.script {
Some(script.as_source())
Some(script.clone())
} else {
hook.command.as_ref().map(|command| {
command
.iter()
.map(InterpString::as_source)
.collect::<Vec<_>>()
.join(" ")
})
hook.command.as_ref().map(|command| join_command(command))
};
HookDefinition {
@ -462,11 +451,24 @@ fn resolve_hook(hook: &HookEntry, index: usize, errors: &mut Vec<ResolveError>)
}
}
/// Join an argv-style `command` into a single space-separated [`InterpString`],
/// preserving every `{{ ... }}` token so the executor resolves it at hook fire
/// time. The join reconstructs the source form once, in one audited place.
#[expect(
clippy::disallowed_methods,
reason = "intentional source preservation: the hook executor re-resolves {{ env.* }} \
tokens at hook fire time"
reason = "deliberate source reconstruction: argv parts are reassembled into one InterpString \
whose tokens stay typed for resolution at hook fire time"
)]
fn join_command(command: &[InterpString]) -> InterpString {
InterpString::parse(
&command
.iter()
.map(InterpString::as_source)
.collect::<Vec<_>>()
.join(" "),
)
}
fn resolve_hook_type(hook: &HookEntry) -> Option<HookType> {
if hook.script.is_some() || hook.command.is_some() {
return None;
@ -479,7 +481,7 @@ fn resolve_hook_type(hook: &HookEntry) -> Option<HookType> {
Some(
hook.headers
.iter()
.map(|(key, value)| (key.clone(), value.as_source()))
.map(|(key, value)| (key.clone(), value.clone()))
.collect(),
)
};
@ -490,7 +492,7 @@ fn resolve_hook_type(hook: &HookEntry) -> Option<HookType> {
None => TlsMode::default(),
};
return Some(HookType::Http {
url: url.as_source(),
url: url.clone(),
headers,
allowed_env_vars: hook.allowed_env_vars.clone(),
tls,
@ -501,17 +503,16 @@ fn resolve_hook_type(hook: &HookEntry) -> Option<HookType> {
return Some(HookType::Agent {
prompt: hook
.prompt
.as_ref()
.map(InterpString::as_source)
.unwrap_or_default(),
model: hook.model.as_ref().map(InterpString::as_source),
.clone()
.unwrap_or_else(|| InterpString::parse("")),
model: hook.model.clone(),
max_tool_rounds: hook.max_tool_rounds,
});
}
hook.prompt.as_ref().map(|prompt| HookType::Prompt {
prompt: prompt.as_source(),
model: hook.model.as_ref().map(InterpString::as_source),
prompt: prompt.clone(),
model: hook.model.clone(),
})
}

30
lib/crates/fabro-hooks/src/config.rs
View file

@ -9,6 +9,7 @@
use std::borrow::Cow;
use fabro_types::settings::InterpString;
use serde::{Deserialize, Serialize};
/// Lifecycle events that can trigger user-defined hooks.
@ -91,23 +92,23 @@ pub enum TlsMode {
#[serde(tag = "type", rename_all = "snake_case")]
pub enum HookType {
Command {
command: String,
command: InterpString,
},
Http {
url: String,
headers: Option<std::collections::HashMap<String, String>>,
url: InterpString,
headers: Option<std::collections::HashMap<String, InterpString>>,
#[serde(default)]
allowed_env_vars: Vec<String>,
#[serde(default)]
tls: TlsMode,
},
Prompt {
prompt: String,
model: Option<String>,
prompt: InterpString,
model: Option<InterpString>,
},
Agent {
prompt: String,
model: Option<String>,
prompt: InterpString,
model: Option<InterpString>,
max_tool_rounds: Option<u32>,
},
}
@ -119,7 +120,7 @@ pub struct HookDefinition {
pub event: HookEvent,
/// Inline command shorthand — if set, implies `type = "command"`.
#[serde(default)]
pub command: Option<String>,
pub command: Option<InterpString>,
/// Explicit hook type (command or http). If omitted and `command` is set,
/// defaults to `Command`.
#[serde(flatten)]
@ -178,6 +179,11 @@ impl HookDefinition {
/// The effective name: explicit name or a generated one.
#[must_use]
#[expect(
clippy::disallowed_methods,
reason = "effective_name builds a human/merge-identity label from the hook's unresolved \
template source; the source text is the intended display value here"
)]
pub fn effective_name(&self) -> String {
if let Some(ref n) = self.name {
return n.clone();
@ -185,12 +191,14 @@ impl HookDefinition {
let event_str = self.event.to_string();
match self.resolved_hook_type().as_deref() {
Some(HookType::Command { ref command }) => {
let short = &command[..command.floor_char_boundary(20)];
let source = command.as_source();
let short = &source[..source.floor_char_boundary(20)];
format!("{event_str}:{short}")
}
Some(HookType::Http { ref url, .. }) => format!("{event_str}:{url}"),
Some(HookType::Http { ref url, .. }) => format!("{event_str}:{}", url.as_source()),
Some(HookType::Prompt { ref prompt, .. } | HookType::Agent { ref prompt, .. }) => {
let short = &prompt[..prompt.floor_char_boundary(20)];
let source = prompt.as_source();
let short = &source[..source.floor_char_boundary(20)];
format!("{event_str}:{short}")
}
None => event_str,

312
lib/crates/fabro-hooks/src/executor.rs
View file

@ -12,7 +12,6 @@ use fabro_llm::client::Client as LlmClient;
use fabro_llm::generate::{GenerateParams, generate_object};
use fabro_llm::types::{Message, Request, ToolResult};
use fabro_model::Catalog;
use fabro_template::{TemplateContext, render as render_template};
use fabro_types::settings::InterpString;
use fabro_util::env::{Env, SystemEnv};
use tokio::process::Command as TokioCommand;
@ -56,28 +55,24 @@ pub trait HookExecutor: Send + Sync {
) -> HookResult;
}
fn resolve_interp_string<E>(value: &str, env: &E) -> Result<String, String>
/// Resolve a typed [`InterpString`] hook segment at fire time, looking up
/// `{{ env.* }}` / `{{ secrets.* }}` tokens against `env`.
///
/// The value stays typed end-to-end: it is carried as an `InterpString`
/// through the config resolve layer and resolved here from its segments —
/// there is no `InterpString -> String -> InterpString` re-parse. A missing or
/// out-of-scope token is a hard error (fail-closed); there is no fallback to
/// the unresolved source.
fn resolve_interp<E>(value: &InterpString, env: &E) -> Result<String, String>
where
E: Env + ?Sized,
{
InterpString::parse(value)
value
.resolve(|name| env.var(name).ok())
.map(|resolved| resolved.value)
.map_err(|error| error.to_string())
}
fn render_header_template<E>(
value: &str,
allowed_vars: &[String],
env: &E,
) -> Result<String, String>
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
{
let ctx = TemplateContext::new().with_env_lookup_allowed(env, allowed_vars);
render_template(value, &ctx).map_err(|error| error.to_string())
}
/// Executes hooks via shell commands or HTTP POST.
pub struct HookExecutorImpl;
@ -105,42 +100,29 @@ impl HookExecutorImpl {
}
}
/// Resolve env vars in the prompt and optional model strings.
/// Returns `None` (with a warning) on resolution failure — callers should
/// proceed when that happens.
/// Resolve the prompt and optional model segments at fire time.
///
/// Fail-closed: a missing or out-of-scope `{{ env.* }}`/`{{ secrets.* }}`
/// token is a hard error so the hook never fires with a half-resolved
/// value. The caller turns the error into a `Block` decision, matching the
/// command-hook behavior.
fn resolve_prompt_and_model<E>(
prompt: &str,
model: Option<&str>,
prompt: &InterpString,
model: Option<&InterpString>,
env: &E,
hook_kind: &str,
) -> Option<(String, Option<String>)>
) -> Result<(String, Option<String>), String>
where
E: Env + ?Sized,
{
let prompt = match resolve_interp_string(prompt, env) {
Ok(prompt) => prompt,
Err(error) => {
tracing::warn!(error = %error, "{hook_kind} hook prompt env resolution failed, proceeding");
return None;
}
};
let model = match model
.map(|model| resolve_interp_string(model, env))
.transpose()
{
Ok(model) => model,
Err(error) => {
tracing::warn!(error = %error, "{hook_kind} hook model env resolution failed, proceeding");
return None;
}
};
Some((prompt, model))
let prompt = resolve_interp(prompt, env)?;
let model = model.map(|model| resolve_interp(model, env)).transpose()?;
Ok((prompt, model))
}
/// Execute a command hook (sandbox or host).
async fn execute_command<E>(
definition: &HookDefinition,
command: &str,
command: &InterpString,
context: &HookContext,
sandbox: &Arc<dyn Sandbox>,
execution_context: &HookExecutionContext,
@ -149,7 +131,7 @@ impl HookExecutorImpl {
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
{
let command = match resolve_interp_string(command, env) {
let command = match resolve_interp(command, env) {
Ok(command) => command,
Err(error) => {
return HookDecision::Block {
@ -298,8 +280,8 @@ impl HookExecutorImpl {
/// Execute a prompt hook: single-turn LLM call returning ok/block.
async fn execute_prompt<E>(
definition: &HookDefinition,
prompt: &str,
model: Option<&str>,
prompt: &InterpString,
model: Option<&InterpString>,
context: &HookContext,
env: &E,
llm_source: &dyn CredentialSource,
@ -308,9 +290,14 @@ impl HookExecutorImpl {
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
{
let Some((prompt, model)) = Self::resolve_prompt_and_model(prompt, model, env, "prompt")
else {
return HookDecision::Proceed;
let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model, env) {
Ok(resolved) => resolved,
Err(error) => {
tracing::error!(error = %error, "prompt hook env resolution failed, not firing");
return HookDecision::Block {
reason: Some(error),
};
}
};
let resolved_model = Self::resolve_model(model.as_deref(), catalog.as_ref());
@ -360,8 +347,8 @@ impl HookExecutorImpl {
/// a normal agent session.
async fn execute_agent<E>(
definition: &HookDefinition,
prompt: &str,
model: Option<&str>,
prompt: &InterpString,
model: Option<&InterpString>,
max_tool_rounds: Option<u32>,
context: &HookContext,
sandbox: Arc<dyn Sandbox>,
@ -372,9 +359,14 @@ impl HookExecutorImpl {
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
{
let Some((prompt, model)) = Self::resolve_prompt_and_model(prompt, model, env, "agent")
else {
return HookDecision::Proceed;
let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model, env) {
Ok(resolved) => resolved,
Err(error) => {
tracing::error!(error = %error, "agent hook env resolution failed, not firing");
return HookDecision::Block {
reason: Some(error),
};
}
};
let resolved_model = Self::resolve_model(model.as_deref(), catalog.as_ref());
@ -492,16 +484,16 @@ impl HookExecutorImpl {
}
/// Execute an HTTP hook: POST context JSON and parse the response.
/// Fail-open: non-2xx and connection errors return `Proceed`.
#[allow(
clippy::too_many_arguments,
reason = "HTTP hook execution needs separate client, TLS, env, and payload inputs."
)]
///
/// Token resolution is fail-closed: a missing or out-of-scope token in the
/// URL or a header is a hard `Block`, so the hook never fires with a
/// half-resolved URL or an empty credential header. Transport outcomes
/// (non-2xx, connection errors, unparseable body) stay fail-open and
/// return `Proceed`.
async fn execute_http<E>(
client: &fabro_http::HttpClient,
url: &str,
headers: Option<&HashMap<String, String>>,
allowed_env_vars: &[String],
url: &InterpString,
headers: Option<&HashMap<String, InterpString>>,
tls: &TlsMode,
context: &HookContext,
timeout: std::time::Duration,
@ -510,15 +502,16 @@ impl HookExecutorImpl {
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
{
let resolved_url = match resolve_interp_string(url, env) {
let resolved_url = match resolve_interp(url, env) {
Ok(url) => url,
Err(error) => {
tracing::warn!(
url = %url,
tracing::error!(
error = %error,
"HTTP hook URL env resolution failed, proceeding"
"HTTP hook URL env resolution failed, not firing"
);
return HookDecision::Proceed;
return HookDecision::Block {
reason: Some(error),
};
}
};
@ -540,16 +533,18 @@ impl HookExecutorImpl {
if let Some(hdrs) = headers {
for (key, value) in hdrs {
let interpolated = match render_header_template(value, allowed_env_vars, env) {
let interpolated = match resolve_interp(value, env) {
Ok(rendered) => rendered,
Err(error) => {
tracing::warn!(
tracing::error!(
url = %resolved_url,
header = %key,
error = %error,
"HTTP hook header template render failed, proceeding"
"HTTP hook header env resolution failed, not firing"
);
return HookDecision::Proceed;
return HookDecision::Block {
reason: Some(error),
};
}
};
request = request.header(key, interpolated);
@ -662,13 +657,13 @@ impl HookExecutor for HookExecutorImpl {
Cow::Borrowed(HookType::Http {
ref url,
ref headers,
ref allowed_env_vars,
allowed_env_vars: _,
ref tls,
})
| Cow::Owned(HookType::Http {
ref url,
ref headers,
ref allowed_env_vars,
allowed_env_vars: _,
ref tls,
}),
) => {
@ -677,7 +672,6 @@ impl HookExecutor for HookExecutorImpl {
clients.get(*tls),
url,
headers.as_ref(),
allowed_env_vars,
tls,
context,
definition.timeout(),
@ -698,7 +692,7 @@ impl HookExecutor for HookExecutorImpl {
Self::execute_prompt(
definition,
prompt,
model.as_deref(),
model.as_ref(),
context,
&env,
llm_source,
@ -721,7 +715,7 @@ impl HookExecutor for HookExecutorImpl {
Self::execute_agent(
definition,
prompt,
model.as_deref(),
model.as_ref(),
*max_tool_rounds,
context,
sandbox,
@ -1053,7 +1047,7 @@ mod tests {
);
}
// --- hook template helpers ---
// --- hook segment resolution helpers ---
fn test_env(vars: &[(&str, &str)]) -> TestEnv {
TestEnv(
@ -1063,45 +1057,49 @@ mod tests {
)
}
fn interp(value: &str) -> InterpString {
InterpString::parse(value)
}
// Headers resolve via the same narrow `{{ ns.NAME }}` token resolver as
// every other hook field — no template engine, no allowlist.
#[test]
fn render_header_template_resolves_allowlisted_var() {
fn header_resolves_narrow_token() {
let env = test_env(&[("FABRO_TEST_KEY_1", "secret123")]);
let result = render_header_template(
"Bearer {{ env.FABRO_TEST_KEY_1 }}",
&["FABRO_TEST_KEY_1".to_string()],
&env,
)
.unwrap();
let result = resolve_interp(&interp("Bearer {{ env.FABRO_TEST_KEY_1 }}"), &env).unwrap();
assert_eq!(result, "Bearer secret123");
}
#[test]
fn render_header_template_rejects_unlisted_var() {
let env = test_env(&[("FABRO_TEST_KEY_3", "should_not_appear")]);
let err = render_header_template("prefix-{{ env.FABRO_TEST_KEY_3 }}-suffix", &[], &env)
.unwrap_err();
assert!(err.contains("undefined"));
fn header_missing_token_is_hard_error() {
let env = test_env(&[]);
let err =
resolve_interp(&interp("prefix-{{ env.FABRO_TEST_KEY_3 }}-suffix"), &env).unwrap_err();
assert!(err.contains("FABRO_TEST_KEY_3"));
}
// The value stays a typed `InterpString`: it resolves at fire time from its
// segments, never via a String -> InterpString re-parse.
#[test]
fn resolve_interp_string_resolves_embedded_var() {
fn resolve_interp_resolves_embedded_token_from_typed_value() {
let env = test_env(&[("FABRO_TEST_KEY_2", "val")]);
let result = resolve_interp_string("x{{ env.FABRO_TEST_KEY_2 }}y", &env).unwrap();
let value = interp("x{{ env.FABRO_TEST_KEY_2 }}y");
let result = resolve_interp(&value, &env).unwrap();
assert_eq!(result, "xvaly");
}
#[test]
fn resolve_interp_string_errors_on_missing_var() {
fn resolve_interp_errors_on_missing_var() {
let env = test_env(&[]);
let err = resolve_interp_string("a{{ env.FABRO_TEST_NOEXIST }}-b", &env).unwrap_err();
let err = resolve_interp(&interp("a{{ env.FABRO_TEST_NOEXIST }}-b"), &env).unwrap_err();
assert!(err.contains("FABRO_TEST_NOEXIST"));
}
#[test]
fn resolve_interp_string_without_vars_passes_through() {
fn resolve_interp_without_tokens_passes_through() {
let env = test_env(&[]);
assert_eq!(
resolve_interp_string("plain text", &env).unwrap(),
resolve_interp(&interp("plain text"), &env).unwrap(),
"plain text"
);
}
@ -1124,9 +1122,8 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
@ -1153,9 +1150,8 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
@ -1180,9 +1176,8 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
@ -1199,9 +1194,8 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
"http://127.0.0.1:1",
&interp("http://127.0.0.1:1"),
None,
&[],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(1),
@ -1228,15 +1222,14 @@ mod tests {
let headers = HashMap::from([(
"Authorization".to_string(),
"Bearer {{ env.FABRO_TEST_TOKEN }}".to_string(),
interp("Bearer {{ env.FABRO_TEST_TOKEN }}"),
)]);
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
Some(&headers),
&["FABRO_TEST_TOKEN".to_string()],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
@ -1262,9 +1255,8 @@ mod tests {
let env = test_env(&[("FABRO_TEST_URL", &server.url("/hook"))]);
let decision = HookExecutorImpl::execute_http(
&client,
"{{ env.FABRO_TEST_URL }}",
&interp("{{ env.FABRO_TEST_URL }}"),
None,
&[],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
@ -1276,6 +1268,76 @@ mod tests {
assert_eq!(decision, HookDecision::Proceed);
}
#[tokio::test]
async fn http_hook_missing_url_token_blocks_without_firing() {
let server = httpmock::MockServer::start_async().await;
let mock = server
.mock_async(|when, then| {
when.method("POST").path("/hook");
then.status(200).body("");
})
.await;
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&interp("{{ env.FABRO_TEST_MISSING_URL }}/hook"),
None,
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
&test_env(&[]),
)
.await;
// Fail-closed: the missing token must not fire the hook at all.
assert_eq!(mock.calls_async().await, 0);
match decision {
HookDecision::Block { reason } => {
assert!(
reason
.as_deref()
.is_some_and(|reason| reason.contains("FABRO_TEST_MISSING_URL")),
"block reason should name the missing token, got: {reason:?}"
);
}
other => panic!("expected Block on missing url token, got {other:?}"),
}
}
#[tokio::test]
async fn http_hook_missing_header_token_blocks_without_firing() {
let server = httpmock::MockServer::start_async().await;
let mock = server
.mock_async(|when, then| {
when.method("POST").path("/hook");
then.status(200).body("");
})
.await;
let headers = HashMap::from([(
"Authorization".to_string(),
interp("Bearer {{ env.FABRO_TEST_MISSING_HEADER }}"),
)]);
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&interp(&server.url("/hook")),
Some(&headers),
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
&test_env(&[]),
)
.await;
// Fail-closed: a missing header token must not fire the hook with an
// empty credential header.
assert_eq!(mock.calls_async().await, 0);
assert!(matches!(decision, HookDecision::Block { .. }));
}
// --- TLS mode enforcement tests ---
#[tokio::test]
@ -1283,9 +1345,8 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
"http://example.com/hook",
&interp("http://example.com/hook"),
None,
&[],
&TlsMode::Verify,
&make_context(),
std::time::Duration::from_secs(5),
@ -1301,9 +1362,8 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
"http://example.com/hook",
&interp("http://example.com/hook"),
None,
&[],
&TlsMode::NoVerify,
&make_context(),
std::time::Duration::from_secs(5),
@ -1327,9 +1387,8 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
@ -1357,7 +1416,7 @@ mod tests {
event: HookEvent::StageStart,
command: None,
hook_type: Some(HookType::Http {
url: server.url("/hook"),
url: interp(&server.url("/hook")),
headers: None,
allowed_env_vars: vec![],
tls: TlsMode::Off,
@ -1391,7 +1450,7 @@ mod tests {
let sandbox = make_sandbox();
let decision = HookExecutorImpl::execute_command(
&make_definition("echo {{ env.MISSING_HOOK_VALUE }}"),
"echo {{ env.MISSING_HOOK_VALUE }}",
&interp("echo {{ env.MISSING_HOOK_VALUE }}"),
&make_context(),
&sandbox,
&HookExecutionContext::default(),
@ -1402,11 +1461,13 @@ mod tests {
assert!(matches!(decision, HookDecision::Block { .. }));
}
// Fail-closed: a prompt hook with a missing token does not fire the LLM
// call; it blocks with the resolution error, matching command hooks.
#[tokio::test]
async fn prompt_hook_missing_env_proceeds() {
async fn prompt_hook_missing_env_blocks() {
let decision = HookExecutorImpl::execute_prompt(
&make_definition("unused"),
"{{ env.MISSING_HOOK_VALUE }}",
&interp("{{ env.MISSING_HOOK_VALUE }}"),
None,
&make_context(),
&test_env(&[]),
@ -1415,14 +1476,25 @@ mod tests {
)
.await;
assert_eq!(decision, HookDecision::Proceed);
match decision {
HookDecision::Block { reason } => {
assert!(
reason
.as_deref()
.is_some_and(|reason| reason.contains("MISSING_HOOK_VALUE")),
"block reason should name the missing token, got: {reason:?}"
);
}
other => panic!("expected Block on missing prompt token, got {other:?}"),
}
}
// Fail-closed: an agent hook with a missing token blocks instead of firing.
#[tokio::test]
async fn agent_hook_missing_env_proceeds() {
async fn agent_hook_missing_env_blocks() {
let decision = HookExecutorImpl::execute_agent(
&make_definition("unused"),
"{{ env.MISSING_HOOK_VALUE }}",
&interp("{{ env.MISSING_HOOK_VALUE }}"),
None,
Some(1),
&make_context(),
@ -1433,6 +1505,6 @@ mod tests {
)
.await;
assert_eq!(decision, HookDecision::Proceed);
assert!(matches!(decision, HookDecision::Block { .. }));
}
}

3
lib/crates/fabro-hooks/src/lib.rs
View file

@ -6,5 +6,8 @@ pub mod types;
pub use bridge::WorkflowToolHookCallback;
pub use config::{HookDefinition, HookSettings, HookType, TlsMode};
// Re-exported because the interpolatable fields of `HookType` are typed as
// `InterpString`; constructing a hook definition requires it.
pub use fabro_types::settings::InterpString;
pub use runner::HookRunner;
pub use types::{HookContext, HookDecision, HookEvent, HookExecutionContext};

4
lib/crates/fabro-hooks/tests/host_command_hooks.rs
View file

@ -5,7 +5,7 @@ use fabro_agent::{LocalSandbox, Sandbox};
use fabro_auth::{CredentialSource, EnvCredentialSource};
use fabro_hooks::{
HookContext, HookDecision, HookDefinition, HookEvent, HookExecutionContext, HookRunner,
HookSettings,
HookSettings, InterpString,
};
use fabro_model::Catalog;
use fabro_types::RunId;
@ -44,7 +44,7 @@ async fn host_command_hook_uses_host_workdir_not_sandbox_workdir() {
hooks: vec![HookDefinition {
name: Some("host-marker".to_string()),
event: HookEvent::RunStart,
command: Some("printf ran > marker.txt".to_string()),
command: Some(InterpString::parse("printf ran > marker.txt")),
hook_type: None,
matcher: None,
blocking: Some(true),

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

@ -117,7 +117,7 @@ impl RunNamespace {
}
for hook in &mut self.hooks {
substitute_option_string(&mut hook.name, &mut lookup)?;
substitute_option_string(&mut hook.command, &mut lookup)?;
substitute_option(&mut hook.command, &mut lookup)?;
substitute_option_string(&mut hook.matcher, &mut lookup)?;
if let Some(hook_type) = &mut hook.hook_type {
substitute_hook_type(hook_type, &mut lookup)?;
@ -320,17 +320,17 @@ where
F: FnMut(&str) -> Option<String>,
{
match hook_type {
HookType::Command { command } => substitute_string(command, lookup),
HookType::Command { command } => substitute(command, lookup),
HookType::Http { url, headers, .. } => {
substitute_string(url, lookup)?;
substitute(url, lookup)?;
if let Some(headers) = headers {
substitute_string_map(headers, lookup)?;
substitute_map(headers, lookup)?;
}
Ok(())
}
HookType::Prompt { prompt, model } | HookType::Agent { prompt, model, .. } => {
substitute_string(prompt, lookup)?;
substitute_option_string(model, lookup)
substitute(prompt, lookup)?;
substitute_option(model, lookup)
}
}
}
@ -386,10 +386,10 @@ mod run_namespace_variable_substitution_tests {
event: HookEvent::RunComplete,
command: None,
hook_type: Some(HookType::Http {
url: "https://hooks.example/{{ vars.ENV }}".to_string(),
url: InterpString::parse("https://hooks.example/{{ vars.ENV }}"),
headers: Some(HashMap::from([(
"X-Env".to_string(),
"{{ vars.ENV }}".to_string(),
InterpString::parse("{{ vars.ENV }}"),
)])),
allowed_env_vars: Vec::new(),
tls: super::TlsMode::Verify,
@ -432,13 +432,13 @@ mod run_namespace_variable_substitution_tests {
}
match run.hooks[0].hook_type.as_ref().unwrap() {
HookType::Http { url, headers, .. } => {
assert_eq!(url, "https://hooks.example/prod");
assert_eq!(url.as_source(), "https://hooks.example/prod");
assert_eq!(
headers
.as_ref()
.and_then(|headers| headers.get("X-Env"))
.map(String::as_str),
Some("prod")
.map(InterpString::as_source),
Some("prod".to_string())
);
}
other => panic!("expected http hook type, got {other:?}"),
@ -1607,23 +1607,23 @@ pub enum TlsMode {
#[serde(tag = "type", rename_all = "snake_case")]
pub enum HookType {
Command {
command: String,
command: InterpString,
},
Http {
url: String,
headers: Option<HashMap<String, String>>,
url: InterpString,
headers: Option<HashMap<String, InterpString>>,
#[serde(default)]
allowed_env_vars: Vec<String>,
#[serde(default)]
tls: TlsMode,
},
Prompt {
prompt: String,
model: Option<String>,
prompt: InterpString,
model: Option<InterpString>,
},
Agent {
prompt: String,
model: Option<String>,
prompt: InterpString,
model: Option<InterpString>,
max_tool_rounds: Option<u32>,
},
}
@ -1633,7 +1633,7 @@ pub struct HookDefinition {
pub name: Option<String>,
pub event: HookEvent,
#[serde(default)]
pub command: Option<String>,
pub command: Option<InterpString>,
#[serde(flatten)]
pub hook_type: Option<HookType>,
pub matcher: Option<String>,
@ -1686,6 +1686,11 @@ impl HookDefinition {
}
#[must_use]
#[expect(
clippy::disallowed_methods,
reason = "effective_name builds a human/merge-identity label from the hook's unresolved \
template source; the source text is the intended display value here"
)]
pub fn effective_name(&self) -> String {
if let Some(ref name) = self.name {
return name.clone();
@ -1693,12 +1698,14 @@ impl HookDefinition {
let event = format!("{:?}", self.event).to_lowercase();
match self.resolved_hook_type().as_deref() {
Some(HookType::Command { command }) => {
let short = &command[..command.floor_char_boundary(20)];
let source = command.as_source();
let short = &source[..source.floor_char_boundary(20)];
format!("{event}:{short}")
}
Some(HookType::Http { url, .. }) => format!("{event}:{url}"),
Some(HookType::Http { url, .. }) => format!("{event}:{}", url.as_source()),
Some(HookType::Prompt { prompt, .. } | HookType::Agent { prompt, .. }) => {
let short = &prompt[..prompt.floor_char_boundary(20)];
let source = prompt.as_source();
let short = &source[..source.floor_char_boundary(20)];
format!("{event}:{short}")
}
None => event,

8
lib/crates/fabro-workflow/tests/it/integration.rs
View file

@ -8906,7 +8906,13 @@ async fn hook_config_merge_run_overrides_by_name() {
let merged = server_hooks.merge(run_hooks);
assert_eq!(merged.hooks.len(), 1);
// Run config wins — command should be "exit 0"
assert_eq!(merged.hooks[0].command.as_deref(), Some("exit 0"));
assert_eq!(
merged.hooks[0]
.command
.as_ref()
.map(fabro_hooks::InterpString::as_source),
Some("exit 0".to_string())
);
// Verify it actually works end-to-end
let engine = engine_with_hooks(merged.hooks);