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

Hooks: typed end-to-end interpolation, narrow header tokens, fail-closed resolution (#528)

Browse source 
## What

Makes hook interpolation typed end-to-end and fail-closed, and removes
the bespoke template engine on HTTP-hook headers.

- **Typed end-to-end.** Hook `command`, `url`, header values, `prompt`,
and `model` are now carried as a typed `InterpString` from the config
resolve layer all the way to the executor. The executor resolves each
segment at hook fire time from the typed value instead of collapsing it
to a `String` and re-parsing it. This mirrors the MCP transport env
resolution boundary (`resolve_transport_env` / `runtime_mcp_server`).
- **Narrow header tokens.** HTTP-hook headers previously ran through
MiniJinja with an env allowlist
(`TemplateContext::with_env_lookup_allowed`). They now resolve through
the same narrow `{{ ns.NAME }}` token resolver as every other hook field
— no template engine, no allowlist.
- **Fail-closed everywhere.** A missing or out-of-scope `{{ env.* }}` /
`{{ secrets.* }}` 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 (warned and proceeded), which could
dispatch an HTTP request with an empty credential header or run an LLM
call against a half-rendered prompt. Transport-level outcomes (non-2xx
responses, connection errors, unparseable bodies) stay fail-open.

A follow-up cleanup commit removes the template engine's `env` namespace
(`with_env_lookup` / `with_env_lookup_allowed` / the `EnvLookup`
object), which the header path was the last consumer of.

## How

- `fabro-types` and `fabro-hooks` `HookType` / `HookDefinition` now type
the interpolatable fields as `InterpString`. `InterpString` serializes
as its raw source, so persisted run specs and checkpoints round-trip
unchanged.
- The `fabro-config` resolve layer clones the typed `InterpString`
through instead of calling `as_source()`, so the fields no longer leak
unresolved template text — the old "source preservation" `#[expect]`
annotations on the hook resolvers are gone.
- The executor's single `resolve_interp` helper resolves a typed
`InterpString` and is shared by the command, http, prompt, and agent
paths; resolution failure maps to `HookDecision::Block`, which the
runner already reports loudly (error for blocking hooks, warn for
non-blocking).

## Testing

- New unit tests: fire-time resolution from the typed value (no
re-parse), narrow-token header resolution, and fail-closed behavior for
HTTP url, HTTP header, and prompt hooks on a missing variable (the hook
does not fire and the resolution error surfaces).
- Existing hook tests updated and kept green.
- Gates: `cargo build --workspace`, `cargo +nightly-2026-04-14 fmt
--check --all`, `cargo +nightly-2026-04-14 clippy --workspace
--all-targets -- -D warnings`, and `cargo nextest run` for the touched
crates (`fabro-hooks`, `fabro-types`, `fabro-config`, `fabro-template`,
`fabro-workflow`, `fabro-server`, and the `fabro-cli` hook/config
tests), all green.

🤖 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-06-30 17:07:11 -04:00 • committed by GitHub
parent 173968a780
commit 287afd7928
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 527 additions and 539 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
Cargo.lock generated
View file

@ -2591,11 +2591,10 @@ dependencies = [
"async-trait",
"fabro-agent",
"fabro-auth",
"fabro-config",
"fabro-http",
"fabro-llm",
"fabro-model",
"fabro-template",
"fabro-redact",
"fabro-types",
"fabro-util",
"httpmock",

9
docs/public/agents/hooks.mdx
View file

@ -28,16 +28,17 @@ POST the event context as JSON to an HTTP endpoint. Useful for webhooks, externa
event = "run_complete"
type = "http"
url = "https://hooks.example.com/done"
allowed_env_vars = ["API_KEY"]
[hooks.headers]
Authorization = "Bearer $API_KEY"
Authorization = "Bearer {{ env.API_KEY }}"
```
| Field | Description |
|---|---|
| `url` | The endpoint to POST to. Must use `https://` unless `tls = "off"`. |
| `headers` | Optional HTTP headers. Values support `$VAR` interpolation from `allowed_env_vars`. |
| `allowed_env_vars` | List of environment variable names that may be interpolated into headers. |
| `url` | The endpoint to POST to. Must use `https://` unless `tls = "off"`. Supports `{{ env.NAME }}` interpolation. |
| `headers` | Optional HTTP headers. Values support `{{ env.NAME }}` interpolation, scoped to the names in `allowed_env_vars`. A token for any other env var fails to resolve and the hook blocks (fail-closed). |
| `allowed_env_vars` | Allowlist of environment variable names a header may read via `{{ env.NAME }}`. Empty (the default) means no env vars may be interpolated into headers. |
| `tls` | TLS mode: `"verify"` (default), `"no_verify"`, or `"off"`. |
### Prompt

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

@ -14225,10 +14225,19 @@ components:
oneOf:
- $ref: "#/components/schemas/StringMap"
- type: "null"
description: >-
Optional HTTP headers for an http hook. Values support
`{{ env.NAME }}` interpolation, scoped to the names listed in
`allowed_env_vars`; a token for any other env var fails to resolve
and the hook blocks (fail-closed).
allowed_env_vars:
type: array
items:
type: string
description: >-
Allowlist of environment variable names that an http hook header may
read via `{{ env.NAME }}`. An empty list (the default) permits no env
vars in headers.
tls:
$ref: "#/components/schemas/TlsMode"
prompt:

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

@ -462,11 +462,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(),
@ -487,15 +482,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 {
@ -512,11 +501,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;
@ -529,7 +531,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(),
)
};
@ -540,7 +542,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,
@ -551,17 +553,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(),
})
}

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

@ -15,10 +15,9 @@ workspace = true
[dependencies]
fabro-agent = { path = "../fabro-agent" }
fabro-auth = { path = "../fabro-auth" }
fabro-config = { path = "../fabro-config" }
fabro-llm = { path = "../fabro-llm" }
fabro-model = { path = "../fabro-model" }
fabro-template = { path = "../fabro-template" }
fabro-redact.workspace = true
fabro-types = { path = "../fabro-types" }
fabro-util = { path = "../fabro-util" }
fabro-http.workspace = true

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

@ -1,203 +1,8 @@
//! Hook configuration runtime types.
//!
//! These types are the runtime shape that the hook executor consumes. The
//! v2 parse tree under `fabro_types::settings::run::HookEntry` is the
//! *config-file* shape; this module lives in `fabro-hooks` because the
//! behavior methods (`is_blocking`, `timeout`, `resolved_hook_type`,
//! `runs_in_sandbox`, `effective_name`) are runtime concerns owned by the
//! executor.
use std::borrow::Cow;
//! Hook configuration runtime settings.
pub use fabro_types::settings::run::{HookDefinition, HookEvent, HookType, TlsMode};
use serde::{Deserialize, Serialize};
/// Lifecycle events that can trigger user-defined hooks.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum HookEvent {
RunStart,
RunComplete,
RunFailed,
StageStart,
StageComplete,
StageFailed,
StageRetrying,
EdgeSelected,
ParallelStart,
ParallelComplete,
/// Reserved: hooks for this event are not yet invoked by the engine.
SandboxReady,
/// Reserved: hooks for this event are not yet invoked by the engine.
SandboxCleanup,
CheckpointSaved,
PreToolUse,
PostToolUse,
PostToolUseFailure,
}
impl HookEvent {
/// Whether hooks for this event block execution by default.
#[must_use]
pub fn is_blocking_by_default(self) -> bool {
matches!(
self,
Self::RunStart
| Self::StageStart
| Self::EdgeSelected
| Self::PreToolUse
| Self::SandboxReady
)
}
}
impl std::fmt::Display for HookEvent {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(match self {
Self::RunStart => "run_start",
Self::RunComplete => "run_complete",
Self::RunFailed => "run_failed",
Self::StageStart => "stage_start",
Self::StageComplete => "stage_complete",
Self::StageFailed => "stage_failed",
Self::StageRetrying => "stage_retrying",
Self::EdgeSelected => "edge_selected",
Self::ParallelStart => "parallel_start",
Self::ParallelComplete => "parallel_complete",
Self::SandboxReady => "sandbox_ready",
Self::SandboxCleanup => "sandbox_cleanup",
Self::CheckpointSaved => "checkpoint_saved",
Self::PreToolUse => "pre_tool_use",
Self::PostToolUse => "post_tool_use",
Self::PostToolUseFailure => "post_tool_use_failure",
})
}
}
/// TLS verification mode for HTTP hooks.
#[derive(Debug, Clone, Copy, Deserialize, PartialEq, Eq, Default, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum TlsMode {
/// Require `https://` and verify certificates (default).
#[default]
Verify,
/// Require `https://` but skip certificate verification.
NoVerify,
/// Allow `http://`; skip certificate verification for `https://`.
Off,
}
/// How a hook is executed.
#[derive(Debug, Clone, Deserialize, PartialEq, Serialize)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum HookType {
Command {
command: String,
},
Http {
url: String,
headers: Option<std::collections::HashMap<String, String>>,
#[serde(default)]
allowed_env_vars: Vec<String>,
#[serde(default)]
tls: TlsMode,
},
Prompt {
prompt: String,
model: Option<String>,
},
Agent {
prompt: String,
model: Option<String>,
max_tool_rounds: Option<u32>,
},
}
/// A single hook definition.
#[derive(Debug, Clone, Deserialize, PartialEq, Serialize)]
pub struct HookDefinition {
pub name: Option<String>,
pub event: HookEvent,
/// Inline command shorthand — if set, implies `type = "command"`.
#[serde(default)]
pub command: Option<String>,
/// Explicit hook type (command or http). If omitted and `command` is set,
/// defaults to `Command`.
#[serde(flatten)]
pub hook_type: Option<HookType>,
/// Regex matched against node_id, handler_type, or event-specific fields.
pub matcher: Option<String>,
/// Override the event's default blocking behavior.
pub blocking: Option<bool>,
/// Timeout in milliseconds (default: 60_000).
pub timeout_ms: Option<u64>,
/// Run inside the sandbox (true, default) or on the host (false).
pub sandbox: Option<bool>,
}
impl HookDefinition {
/// Resolve the effective hook type: explicit `hook_type` wins, then
/// `command` shorthand, then error.
pub fn resolved_hook_type(&self) -> Option<Cow<'_, HookType>> {
if let Some(ref ht) = self.hook_type {
return Some(Cow::Borrowed(ht));
}
self.command.as_ref().map(|cmd| {
Cow::Owned(HookType::Command {
command: cmd.clone(),
})
})
}
/// Whether this hook is blocking for its event.
#[must_use]
pub fn is_blocking(&self) -> bool {
self.blocking
.unwrap_or_else(|| self.event.is_blocking_by_default())
}
/// Timeout duration for this hook.
///
/// Defaults: 30s for prompt hooks, 60s for all others.
#[must_use]
pub fn timeout(&self) -> std::time::Duration {
if let Some(ms) = self.timeout_ms {
return std::time::Duration::from_millis(ms);
}
let default_ms = match self.resolved_hook_type().as_deref() {
Some(HookType::Prompt { .. }) => 30_000,
_ => 60_000,
};
std::time::Duration::from_millis(default_ms)
}
/// Whether this hook runs in the sandbox.
#[must_use]
pub fn runs_in_sandbox(&self) -> bool {
self.sandbox.unwrap_or(true)
}
/// The effective name: explicit name or a generated one.
#[must_use]
pub fn effective_name(&self) -> String {
if let Some(ref n) = self.name {
return n.clone();
}
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)];
format!("{event_str}:{short}")
}
Some(HookType::Http { ref url, .. }) => format!("{event_str}:{url}"),
Some(HookType::Prompt { ref prompt, .. } | HookType::Agent { ref prompt, .. }) => {
let short = &prompt[..prompt.floor_char_boundary(20)];
format!("{event_str}:{short}")
}
None => event_str,
}
}
}
/// Top-level hook configuration: a list of hook definitions.
#[derive(Debug, Clone, Default, Deserialize, PartialEq, Serialize)]
pub struct HookSettings {

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

@ -12,8 +12,9 @@ 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_redact::redacted_url_for_log;
use fabro_types::settings::interp::Namespace;
use fabro_types::settings::{InterpString, ResolveError};
use fabro_util::env::{Env, SystemEnv};
use tokio::process::Command as TokioCommand;
use tokio::time::timeout as tokio_timeout;
@ -56,26 +57,99 @@ 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.* }}` tokens against `env`.
///
/// Only the `env` namespace is wired here; `{{ secrets.* }}`, `{{ vars.* }}`,
/// and `{{ inputs.* }}` tokens have no lookup in this context and resolve as
/// `Unavailable`, which is a hard error — so a hook that references one fails
/// closed rather than firing with a half-resolved value.
///
/// 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.
///
/// Returns the typed [`ResolveError`] so callers keep the source until the
/// decision boundary renders it; do not flatten it to a `String` here.
fn resolve_interp<E>(value: &InterpString, env: &E) -> Result<String, ResolveError>
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],
#[expect(
clippy::disallowed_methods,
reason = "hook HTTP logs use the unresolved token source, not the resolved URL, so env-sourced \
URL material is not logged; redacted_url_for_log masks literal credentials in \
parseable source URLs and replaces unparseable sources with a placeholder"
)]
fn safe_url_source_for_log(url: &InterpString) -> String {
redacted_url_for_log(&url.as_source())
}
#[derive(Debug, Clone, PartialEq, Eq)]
enum HeaderResolveError {
NotAllowed { name: String },
Resolve(ResolveError),
}
impl fmt::Display for HeaderResolveError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::NotAllowed { name } => write!(
f,
"environment variable {name:?} referenced by an HTTP hook header is not listed in \
allowed_env_vars"
),
Self::Resolve(error) => error.fmt(f),
}
}
}
impl std::error::Error for HeaderResolveError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::NotAllowed { .. } => None,
Self::Resolve(error) => Some(error),
}
}
}
/// Resolve an HTTP-hook **header** value at fire time, scoping its
/// `{{ env.* }}` lookups to `allowed_env_vars`.
///
/// Headers carry credentials, so unlike every other hook field they read env
/// through an allowlist: a `{{ env.NAME }}` token resolves only when `NAME` is
/// listed in the hook's `allowed_env_vars`. A name outside the allowlist fails
/// with a distinct error before any lookup, while an allowlisted-but-unset name
/// still surfaces as the normal `Missing` error. An empty `allowed_env_vars`
/// therefore permits no env vars in headers at all. This mirrors the previous
/// template-based `with_env_lookup_allowed` behavior without reviving any
/// template engine.
fn resolve_header<E>(
value: &InterpString,
allowed_env_vars: &[String],
env: &E,
) -> Result<String, String>
) -> Result<String, HeaderResolveError>
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
E: Env + ?Sized,
{
let ctx = TemplateContext::new().with_env_lookup_allowed(env, allowed_vars);
render_template(value, &ctx).map_err(|error| error.to_string())
if let Some(name) = value.names(Namespace::Env).into_iter().find(|name| {
!allowed_env_vars
.iter()
.any(|allowed| allowed.as_str() == *name)
}) {
return Err(HeaderResolveError::NotAllowed {
name: name.to_string(),
});
}
resolve_interp(value, env).map_err(HeaderResolveError::Resolve)
}
/// Executes hooks via shell commands or HTTP POST.
@ -105,55 +179,42 @@ 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: only `{{ env.* }}` is wired here; a missing env token (or a
/// token in any other, unavailable namespace) 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>), ResolveError>
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,
env: &E,
) -> HookDecision
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
E: Env + ?Sized,
{
let command = match resolve_interp_string(command, env) {
let command = match resolve_interp(command, env) {
Ok(command) => command,
Err(error) => {
return HookDecision::Block {
reason: Some(error),
reason: Some(error.to_string()),
};
}
};
@ -298,19 +359,24 @@ 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,
catalog: Arc<Catalog>,
) -> HookDecision
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
E: Env + ?Sized,
{
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.to_string()),
};
}
};
let resolved_model = Self::resolve_model(model.as_deref(), catalog.as_ref());
@ -360,8 +426,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>,
@ -370,11 +436,16 @@ impl HookExecutorImpl {
catalog: Arc<Catalog>,
) -> HookDecision
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
E: Env + ?Sized,
{
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.to_string()),
};
}
};
let resolved_model = Self::resolve_model(model.as_deref(), catalog.as_ref());
@ -492,15 +563,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>>,
url: &InterpString,
headers: Option<&HashMap<String, InterpString>>,
allowed_env_vars: &[String],
tls: &TlsMode,
context: &HookContext,
@ -508,17 +580,19 @@ impl HookExecutorImpl {
env: &E,
) -> HookDecision
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
E: Env + ?Sized,
{
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!(
url_source = %safe_url_source_for_log(url),
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.to_string()),
};
}
};
@ -540,16 +614,22 @@ impl HookExecutorImpl {
if let Some(hdrs) = headers {
for (key, value) in hdrs {
let interpolated = match render_header_template(value, allowed_env_vars, env) {
// Headers resolve through the per-hook env allowlist: a
// `{{ env.NAME }}` not in `allowed_env_vars` blocks before any
// lookup, while an allowlisted-but-unset name still fails as
// missing.
let interpolated = match resolve_header(value, allowed_env_vars, env) {
Ok(rendered) => rendered,
Err(error) => {
tracing::warn!(
url = %resolved_url,
tracing::error!(
url_source = %safe_url_source_for_log(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.to_string()),
};
}
};
request = request.header(key, interpolated);
@ -559,14 +639,18 @@ impl HookExecutorImpl {
let response = match request.send().await {
Ok(resp) => resp,
Err(e) => {
tracing::warn!(url = %resolved_url, error = %e, "HTTP hook request failed, proceeding");
tracing::warn!(
url_source = %safe_url_source_for_log(url),
error = %e,
"HTTP hook request failed, proceeding"
);
return HookDecision::Proceed;
}
};
if !response.status().is_success() {
tracing::warn!(
url = %resolved_url,
url_source = %safe_url_source_for_log(url),
status = response.status().as_u16(),
"HTTP hook returned non-2xx, proceeding"
);
@ -576,7 +660,11 @@ impl HookExecutorImpl {
let body = match response.text().await {
Ok(text) => text,
Err(e) => {
tracing::warn!(url = %resolved_url, error = %e, "HTTP hook body read failed, proceeding");
tracing::warn!(
url_source = %safe_url_source_for_log(url),
error = %e,
"HTTP hook body read failed, proceeding"
);
return HookDecision::Proceed;
}
};
@ -588,7 +676,11 @@ impl HookExecutorImpl {
match serde_json::from_str::<HookDecision>(body.trim()) {
Ok(decision) => decision,
Err(e) => {
tracing::warn!(url = %resolved_url, error = %e, "HTTP hook response parse failed, proceeding");
tracing::warn!(
url_source = %safe_url_source_for_log(url),
error = %e,
"HTTP hook response parse failed, proceeding"
);
HookDecision::Proceed
}
}
@ -698,7 +790,7 @@ impl HookExecutor for HookExecutorImpl {
Self::execute_prompt(
definition,
prompt,
model.as_deref(),
model.as_ref(),
context,
&env,
llm_source,
@ -721,7 +813,7 @@ impl HookExecutor for HookExecutorImpl {
Self::execute_agent(
definition,
prompt,
model.as_deref(),
model.as_ref(),
*max_tool_rounds,
context,
sandbox,
@ -1053,7 +1145,7 @@ mod tests {
);
}
// --- hook template helpers ---
// --- hook segment resolution helpers ---
fn test_env(vars: &[(&str, &str)]) -> TestEnv {
TestEnv(
@ -1063,11 +1155,37 @@ mod tests {
)
}
fn interp(value: &str) -> InterpString {
InterpString::parse(value)
}
#[test]
fn render_header_template_resolves_allowlisted_var() {
fn safe_url_source_for_log_redacts_parseable_url_source() {
let safe = safe_url_source_for_log(&interp(
"https://user:secret@example.com/hook?token=literal&keep=value",
));
assert_eq!(
safe,
"https://user:****@example.com/hook?token=****&keep=value"
);
}
#[test]
fn safe_url_source_for_log_hides_unparseable_url_source() {
let safe = safe_url_source_for_log(&interp("{{ env.FABRO_TEST_HOOK_URL }}"));
assert_eq!(safe, "<invalid url>");
}
// Headers resolve `{{ env.NAME }}` tokens through the per-hook
// `allowed_env_vars` allowlist: an allowlisted name resolves, anything else
// fails closed before lookup.
#[test]
fn header_resolves_allowlisted_var() {
let env = test_env(&[("FABRO_TEST_KEY_1", "secret123")]);
let result = render_header_template(
"Bearer {{ env.FABRO_TEST_KEY_1 }}",
let result = resolve_header(
&interp("Bearer {{ env.FABRO_TEST_KEY_1 }}"),
&["FABRO_TEST_KEY_1".to_string()],
&env,
)
@ -1075,33 +1193,62 @@ mod tests {
assert_eq!(result, "Bearer secret123");
}
// Fail-closed: a header may not read an env var that is set in the process
// but missing from `allowed_env_vars`. This is distinct from an unset
// allowlisted variable, so the block reason points at the allowlist.
#[test]
fn render_header_template_rejects_unlisted_var() {
fn header_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"));
let err = resolve_header(
&interp("prefix-{{ env.FABRO_TEST_KEY_3 }}-suffix"),
&[],
&env,
)
.unwrap_err();
assert_eq!(err, HeaderResolveError::NotAllowed {
name: "FABRO_TEST_KEY_3".to_string(),
});
}
#[test]
fn resolve_interp_string_resolves_embedded_var() {
fn header_missing_token_is_hard_error() {
let env = test_env(&[]);
let err = resolve_header(
&interp("prefix-{{ env.FABRO_TEST_KEY_3 }}-suffix"),
&["FABRO_TEST_KEY_3".to_string()],
&env,
)
.unwrap_err();
match err {
HeaderResolveError::Resolve(error) => assert_eq!(error.name, "FABRO_TEST_KEY_3"),
HeaderResolveError::NotAllowed { .. } => {
panic!("expected missing token resolve error, got {err:?}")
}
}
}
// 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_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();
assert!(err.contains("FABRO_TEST_NOEXIST"));
let err = resolve_interp(&interp("a{{ env.FABRO_TEST_NOEXIST }}-b"), &env).unwrap_err();
assert_eq!(err.name, "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,7 +1271,7 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
@ -1153,7 +1300,7 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
@ -1180,7 +1327,7 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
@ -1199,7 +1346,7 @@ 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,
@ -1228,13 +1375,13 @@ 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,
@ -1248,6 +1395,53 @@ mod tests {
assert_eq!(decision, HookDecision::Proceed);
}
// Fail-closed: a header that references an env var set in the process but
// absent from `allowed_env_vars` must block and never fire the request.
#[tokio::test]
async fn http_hook_unlisted_header_var_blocks_without_firing() {
let env = test_env(&[("FABRO_TEST_TOKEN", "my-secret")]);
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_TOKEN }}"),
)]);
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&interp(&server.url("/hook")),
Some(&headers),
// Empty allowlist: the env var is set, but headers may read nothing.
&[],
&TlsMode::Off,
&make_context(),
std::time::Duration::from_secs(5),
&env,
)
.await;
assert_eq!(mock.calls_async().await, 0);
match decision {
HookDecision::Block { reason } => {
assert!(
reason
.as_deref()
.is_some_and(|reason| reason.contains("FABRO_TEST_TOKEN")),
"block reason should name the unlisted token, got: {reason:?}"
);
}
other => panic!("expected Block on unlisted header var, got {other:?}"),
}
}
#[tokio::test]
async fn http_hook_resolves_url_before_dispatch() {
let server = httpmock::MockServer::start_async().await;
@ -1262,7 +1456,7 @@ 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,
@ -1276,6 +1470,79 @@ 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),
// Allowlisted but unset: still blocks on the Missing lookup.
&["FABRO_TEST_MISSING_HEADER".to_string()],
&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,7 +1550,7 @@ 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,
@ -1301,7 +1568,7 @@ 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,
@ -1327,7 +1594,7 @@ mod tests {
let client = test_http_client();
let decision = HookExecutorImpl::execute_http(
&client,
&server.url("/hook"),
&interp(&server.url("/hook")),
None,
&[],
&TlsMode::Off,
@ -1357,7 +1624,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 +1658,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 +1669,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 +1684,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 +1713,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),

27
lib/crates/fabro-redact/src/lib.rs
View file

@ -12,6 +12,20 @@ mod safe_url;
pub use jsonl::{redact_json_value, redact_jsonl_line};
pub use safe_url::{DisplaySafeUrl, DisplaySafeUrlError};
/// Redact a URL string for log or error output.
///
/// Returns the credential-redacted form when `url` parses as a URL, or a fixed
/// `"<invalid url>"` placeholder when it does not. This is the one place log
/// sites should reach for instead of re-rolling the
/// [`DisplaySafeUrl::parse`] + [`DisplaySafeUrl::redacted_string`] fallback
/// themselves, so an unparseable or credential-bearing URL never leaks into a
/// log line.
#[must_use]
pub fn redacted_url_for_log(url: &str) -> String {
DisplaySafeUrl::parse(url)
.map_or_else(|_| "<invalid url>".to_string(), |url| url.redacted_string())
}
/// A byte range within a string that should be redacted.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Region {
@ -69,6 +83,19 @@ mod tests {
assert_eq!(redact_string("hello world"), "hello world");
}
#[test]
fn redacted_url_for_log_redacts_credentials() {
assert_eq!(
redacted_url_for_log("https://user:secret@example.com/hook?token=literal&keep=value"),
"https://user:****@example.com/hook?token=****&keep=value"
);
}
#[test]
fn redacted_url_for_log_uses_placeholder_when_unparseable() {
assert_eq!(redacted_url_for_log("{{ env.HOOK_URL }}"), "<invalid url>");
}
#[test]
fn redact_string_with_aws_key() {
let result = redact_string("key=AKIAYRWQG5EJLPZLBYNP");

92
lib/crates/fabro-template/src/lib.rs
View file

@ -3,7 +3,6 @@ use std::fmt;
use std::sync::{Arc, Mutex};
use fabro_types::ManifestPath;
use fabro_util::env::Env;
use miette::{LabeledSpan, NamedSource, SourceCode, SourceSpan};
use minijinja::value::{Object, Value};
use minijinja::{AutoEscape, Environment, ErrorKind, UndefinedBehavior};
@ -53,7 +52,6 @@ pub struct TemplateContext {
goal: Option<String>,
inputs: Value,
vars: Value,
env: Option<Value>,
}
impl Default for TemplateContext {
@ -62,7 +60,6 @@ impl Default for TemplateContext {
goal: None,
inputs: Value::from_serialize(HashMap::<String, toml::Value>::new()),
vars: Value::from_serialize(HashMap::<String, String>::new()),
env: None,
}
}
}
@ -100,41 +97,11 @@ impl TemplateContext {
Self::new().with_goal("{{ goal }}").with_inputs(inputs)
}
#[must_use]
pub fn with_env_lookup<E>(mut self, env: &E) -> Self
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
{
self.env = Some(Value::from_object(EnvLookup {
env: env.clone(),
allowlist: None,
}));
self
}
#[must_use]
pub fn with_env_lookup_allowed<E>(mut self, env: &E, allowlist: &[String]) -> Self
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
{
self.env = Some(Value::from_object(EnvLookup {
env: env.clone(),
allowlist: Some(allowlist.to_vec()),
}));
self
}
fn into_value(self) -> Value {
let goal = self.goal.map(Value::from);
let inputs = self.inputs;
let vars = self.vars;
let env = self.env;
Value::from_object(RenderContext {
goal,
inputs,
vars,
env,
})
Value::from_object(RenderContext { goal, inputs, vars })
}
}
@ -143,7 +110,6 @@ struct RenderContext {
goal: Option<Value>,
inputs: Value,
vars: Value,
env: Option<Value>,
}
impl Object for RenderContext {
@ -152,33 +118,11 @@ impl Object for RenderContext {
"goal" => self.goal.clone(),
"inputs" => Some(self.inputs.clone()),
"vars" => Some(self.vars.clone()),
"env" => self.env.clone(),
_ => None,
}
}
}
#[derive(Debug, Clone)]
pub struct EnvLookup<E> {
env: E,
allowlist: Option<Vec<String>>,
}
impl<E> Object for EnvLookup<E>
where
E: Env + Send + Sync + fmt::Debug + 'static,
{
fn get_value_by_str(self: &Arc<Self>, key: &str) -> Option<Value> {
if let Some(allowlist) = &self.allowlist {
if !allowlist.iter().any(|allowed| allowed == key) {
return None;
}
}
self.env.var(key).ok().map(Value::from)
}
}
/// Errors from rendering a template. Each variant carries the typed fields
/// MiniJinja knows about (offending expression, line) plus the original
/// `minijinja::Error` as `#[source]`, so the cause chain is preserved across
@ -815,7 +759,6 @@ fn reject_loader_dependent_string(name: Option<&str>, template: &str) -> Result<
mod tests {
use std::collections::HashMap;
use fabro_util::env::TestEnv;
use fabro_util::error;
use toml::map::Map;
@ -916,39 +859,6 @@ mod tests {
assert_eq!(rendered, "Repo fabro");
}
#[test]
fn renders_env_variable() {
let env = TestEnv(HashMap::from([(
"API_KEY".to_string(),
"secret".to_string(),
)]));
let ctx = TemplateContext::new().with_env_lookup(&env);
let rendered = render("{{ env.API_KEY }}", &ctx).unwrap();
assert_eq!(rendered, "secret");
}
#[test]
fn renders_allowlisted_env_variable() {
let env = TestEnv(HashMap::from([("TOKEN".to_string(), "abc123".to_string())]));
let ctx = TemplateContext::new().with_env_lookup_allowed(&env, &["TOKEN".to_string()]);
let rendered = render("Bearer {{ env.TOKEN }}", &ctx).unwrap();
assert_eq!(rendered, "Bearer abc123");
}
#[test]
fn rejects_non_allowlisted_env_variable() {
let env = TestEnv(HashMap::from([("SECRET".to_string(), "shh".to_string())]));
let ctx = TemplateContext::new().with_env_lookup_allowed(&env, &[]);
let err = render("{{ env.SECRET }}", &ctx).unwrap_err();
assert!(matches!(err, TemplateError::UndefinedVariable { .. }));
}
#[test]
fn render_lenient_treats_undefined_as_empty() {
let ctx = TemplateContext::new();

82
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>,
@ -1656,16 +1656,8 @@ impl HookDefinition {
#[must_use]
pub fn is_blocking(&self) -> bool {
self.blocking.unwrap_or({
matches!(
self.event,
HookEvent::RunStart
| HookEvent::StageStart
| HookEvent::EdgeSelected
| HookEvent::PreToolUse
| HookEvent::SandboxReady
)
})
self.blocking
.unwrap_or_else(|| self.event.is_blocking_by_default())
}
#[must_use]
@ -1686,19 +1678,26 @@ 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();
}
let event = format!("{:?}", self.event).to_lowercase();
let event = self.event.to_string();
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,
@ -1798,8 +1797,9 @@ pub enum AgentPermissions {
Full,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, strum::Display)]
#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "snake_case")]
pub enum HookEvent {
RunStart,
RunComplete,
@ -1819,6 +1819,20 @@ pub enum HookEvent {
PostToolUseFailure,
}
impl HookEvent {
#[must_use]
pub fn is_blocking_by_default(self) -> bool {
matches!(
self,
Self::RunStart
| Self::StageStart
| Self::EdgeSelected
| Self::PreToolUse
| Self::SandboxReady
)
}
}
#[derive(
Debug,
Clone,

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

@ -16,10 +16,9 @@ use fabro_sandbox::from_environment::{
use fabro_sandbox::{DockerSandboxOptions, SandboxSpec};
use fabro_static::EnvVars;
use fabro_types::settings::run::{
ApprovalMode, HookDefinition as ResolvedHookDefinition, HookEvent as ResolvedHookEvent,
HookType as ResolvedHookType, McpServerSettings as ResolvedMcpServerSettings,
PullRequestSettings, ResolvedMcpEntry, RunMode, RunModelSettings as ResolvedRunModelSettings,
RunNamespace as ResolvedRunSettings, TlsMode as ResolvedTlsMode,
ApprovalMode, McpServerSettings as ResolvedMcpServerSettings, PullRequestSettings,
ResolvedMcpEntry, RunMode, RunModelSettings as ResolvedRunModelSettings,
RunNamespace as ResolvedRunSettings,
};
use fabro_types::settings::{ModelRegistry, ResolvedModelRef};
use fabro_types::{ManifestPath, RunId, RunRunnableSource, SandboxProviderKind};
@ -476,7 +475,7 @@ impl RunSession {
setup_command_timeout_ms: resolved.prepare.timeout_ms,
},
hooks: fabro_hooks::HookSettings {
hooks: resolved.hooks.iter().map(runtime_hook_definition).collect(),
hooks: resolved.hooks.clone(),
},
sandbox_env,
seed_context: None,
@ -704,72 +703,6 @@ fn runtime_mcp_server(
})
}
fn runtime_hook_definition(definition: &ResolvedHookDefinition) -> fabro_hooks::HookDefinition {
fabro_hooks::HookDefinition {
name: definition.name.clone(),
event: match definition.event {
ResolvedHookEvent::RunStart => fabro_hooks::HookEvent::RunStart,
ResolvedHookEvent::RunComplete => fabro_hooks::HookEvent::RunComplete,
ResolvedHookEvent::RunFailed => fabro_hooks::HookEvent::RunFailed,
ResolvedHookEvent::StageStart => fabro_hooks::HookEvent::StageStart,
ResolvedHookEvent::StageComplete => fabro_hooks::HookEvent::StageComplete,
ResolvedHookEvent::StageFailed => fabro_hooks::HookEvent::StageFailed,
ResolvedHookEvent::StageRetrying => fabro_hooks::HookEvent::StageRetrying,
ResolvedHookEvent::EdgeSelected => fabro_hooks::HookEvent::EdgeSelected,
ResolvedHookEvent::ParallelStart => fabro_hooks::HookEvent::ParallelStart,
ResolvedHookEvent::ParallelComplete => fabro_hooks::HookEvent::ParallelComplete,
ResolvedHookEvent::SandboxReady => fabro_hooks::HookEvent::SandboxReady,
ResolvedHookEvent::SandboxCleanup => fabro_hooks::HookEvent::SandboxCleanup,
ResolvedHookEvent::CheckpointSaved => fabro_hooks::HookEvent::CheckpointSaved,
ResolvedHookEvent::PreToolUse => fabro_hooks::HookEvent::PreToolUse,
ResolvedHookEvent::PostToolUse => fabro_hooks::HookEvent::PostToolUse,
ResolvedHookEvent::PostToolUseFailure => fabro_hooks::HookEvent::PostToolUseFailure,
},
command: definition.command.clone(),
hook_type: definition.hook_type.as_ref().map(runtime_hook_type),
matcher: definition.matcher.clone(),
blocking: definition.blocking,
timeout_ms: definition.timeout_ms,
sandbox: definition.sandbox,
}
}
fn runtime_hook_type(hook_type: &ResolvedHookType) -> fabro_hooks::HookType {
match hook_type {
ResolvedHookType::Command { command } => fabro_hooks::HookType::Command {
command: command.clone(),
},
ResolvedHookType::Http {
url,
headers,
allowed_env_vars,
tls,
} => fabro_hooks::HookType::Http {
url: url.clone(),
headers: headers.clone(),
allowed_env_vars: allowed_env_vars.clone(),
tls: match tls {
ResolvedTlsMode::Verify => fabro_hooks::TlsMode::Verify,
ResolvedTlsMode::NoVerify => fabro_hooks::TlsMode::NoVerify,
ResolvedTlsMode::Off => fabro_hooks::TlsMode::Off,
},
},
ResolvedHookType::Prompt { prompt, model } => fabro_hooks::HookType::Prompt {
prompt: prompt.clone(),
model: model.clone(),
},
ResolvedHookType::Agent {
prompt,
model,
max_tool_rounds,
} => fabro_hooks::HookType::Agent {
prompt: prompt.clone(),
model: model.clone(),
max_tool_rounds: *max_tool_rounds,
},
}
}
impl RunSession {
/// Shared engine: initialize, execute, finalize, pull_request.
async fn run(

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);