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

Add HTTP hook executor with env var interpolation

Browse source 
HTTP hooks (type = "http") now actually execute instead of failing with
"no command specified". The executor POSTs the hook context as JSON,
parses HookDecision from the response, and fails open on errors.

Header values support $VAR/${VAR} interpolation gated by an
allowed_env_vars whitelist on the hook definition. Renames
CommandHookExecutor to HookExecutorImpl since it now handles both
command and HTTP hook types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-05 01:59:38 -05:00
parent fd70d41fca
commit 97594b3ab0
5 changed files with 474 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
Cargo.lock generated
View file

@ -335,10 +335,12 @@ dependencies = [
"futures",
"git2",
"indicatif",
"mockito",
"nom",
"predicates",
"rand 0.8.5",
"regex",
"reqwest 0.12.28",
"scopeguard",
"serde",
"serde_json",
@ -776,6 +778,15 @@ version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b05b61dc5112cbb17e4b6cd61790d9845d13888356391624cbe7e41efeac1e75"
[[package]]
name = "colored"
version = "3.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34"
dependencies = [
"windows-sys 0.61.2",
]
[[package]]
name = "combine"
version = "4.6.7"
@ -2557,6 +2568,31 @@ dependencies = [
"windows-sys 0.61.2",
]
[[package]]
name = "mockito"
version = "1.7.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "90820618712cab19cfc46b274c6c22546a82affcb3c3bdf0f29e3db8e1bb92c0"
dependencies = [
"assert-json-diff",
"bytes",
"colored",
"futures-core",
"http",
"http-body",
"http-body-util",
"hyper",
"hyper-util",
"log",
"pin-project-lite",
"rand 0.9.2",
"regex",
"serde_json",
"serde_urlencoded",
"similar",
"tokio",
]
[[package]]
name = "native-tls"
version = "0.2.18"

2
crates/arc-workflows/Cargo.toml
View file

@ -44,7 +44,9 @@ console.workspace = true
indicatif.workspace = true
tokio-util.workspace = true
tracing.workspace = true
reqwest.workspace = true
[dev-dependencies]
mockito = "1"
tokio = { workspace = true, features = ["test-util", "macros"] }
tempfile = "3"
dotenvy.workspace = true

58
crates/arc-workflows/src/hook/config.rs
View file

@ -7,7 +7,12 @@ use super::types::HookEvent;
#[serde(tag = "type", rename_all = "snake_case")]
pub enum HookType {
Command { command: String },
Http { url: String, headers: Option<std::collections::HashMap<String, String>> },
Http {
url: String,
headers: Option<std::collections::HashMap<String, String>>,
#[serde(default)]
allowed_env_vars: Vec<String>,
},
}
/// A single hook definition.
@ -170,6 +175,57 @@ url = "https://hooks.example.com/done"
));
}
#[test]
fn parse_http_hook_with_allowed_env_vars() {
let toml = r#"
[[hooks]]
event = "run_start"
type = "http"
url = "https://hooks.example.com/start"
allowed_env_vars = ["API_KEY", "SECRET"]
[hooks.headers]
Authorization = "Bearer $API_KEY"
"#;
let config: HookConfig = toml::from_str(toml).unwrap();
let hook = &config.hooks[0];
match hook.resolved_hook_type().unwrap() {
HookType::Http {
url,
headers,
allowed_env_vars,
} => {
assert_eq!(url, "https://hooks.example.com/start");
assert_eq!(allowed_env_vars, vec!["API_KEY", "SECRET"]);
assert_eq!(
headers.unwrap().get("Authorization").unwrap(),
"Bearer $API_KEY"
);
}
_ => panic!("expected Http hook type"),
}
}
#[test]
fn parse_http_hook_allowed_env_vars_defaults_empty() {
let toml = r#"
[[hooks]]
event = "run_complete"
type = "http"
url = "https://hooks.example.com/done"
"#;
let config: HookConfig = toml::from_str(toml).unwrap();
let hook = &config.hooks[0];
match hook.resolved_hook_type().unwrap() {
HookType::Http {
allowed_env_vars, ..
} => {
assert!(allowed_env_vars.is_empty());
}
_ => panic!("expected Http hook type"),
}
}
#[test]
fn parse_full_hook_definition() {
let toml = r#"

425
crates/arc-workflows/src/hook/executor.rs
View file

@ -6,7 +6,7 @@ use async_trait::async_trait;
use arc_agent::Sandbox;
use super::config::HookDefinition;
use super::config::{HookDefinition, HookType};
use super::types::{HookContext, HookDecision, HookResult};
/// Trait for executing hooks via different transports.
@ -21,10 +21,51 @@ pub trait HookExecutor: Send + Sync {
) -> HookResult;
}
/// Executes hooks as shell commands (host or sandbox).
pub struct CommandHookExecutor;
/// Interpolate `$VAR` and `${VAR}` references in `value` using environment
/// variables, but only when the variable name appears in `allowed_vars`.
/// Unlisted or missing vars are replaced with the empty string.
pub fn interpolate_env_vars(value: &str, allowed_vars: &[String]) -> String {
let mut result = String::with_capacity(value.len());
let mut chars = value.chars().peekable();
impl CommandHookExecutor {
while let Some(ch) = chars.next() {
if ch == '$' {
let braced = chars.peek() == Some(&'{');
if braced {
chars.next(); // consume '{'
}
let mut var_name = String::new();
while let Some(&c) = chars.peek() {
if braced {
if c == '}' {
chars.next();
break;
}
} else if !c.is_ascii_alphanumeric() && c != '_' {
break;
}
var_name.push(c);
chars.next();
}
if !var_name.is_empty() && allowed_vars.iter().any(|v| v == &var_name) {
if let Ok(val) = std::env::var(&var_name) {
result.push_str(&val);
}
}
} else {
result.push(ch);
}
}
result
}
/// Executes hooks via shell commands or HTTP POST.
pub struct HookExecutorImpl;
impl HookExecutorImpl {
/// Parse a hook decision from JSON stdout and exit code.
fn parse_decision(exit_code: i32, stdout: &str) -> HookDecision {
if exit_code == 0 {
@ -47,31 +88,15 @@ impl CommandHookExecutor {
}
}
}
}
#[async_trait]
impl HookExecutor for CommandHookExecutor {
async fn execute(
&self,
/// Execute a command hook (sandbox or host).
async fn execute_command(
definition: &HookDefinition,
command: &str,
context: &HookContext,
sandbox: &dyn Sandbox,
work_dir: Option<&Path>,
) -> HookResult {
let start = Instant::now();
let command = match definition.resolved_hook_type() {
Some(super::config::HookType::Command { ref command }) => command.clone(),
_ => {
return HookResult {
hook_name: definition.name.clone(),
decision: HookDecision::Block {
reason: Some("no command specified".into()),
},
duration_ms: 0,
};
}
};
) -> HookDecision {
let context_json = serde_json::to_string(context).unwrap_or_default();
let timeout_ms = definition.timeout().as_millis() as u64;
@ -83,8 +108,7 @@ impl HookExecutor for CommandHookExecutor {
env_vars.insert("ARC_NODE_ID".to_string(), node_id.clone());
}
let decision = if definition.runs_in_sandbox() {
// Write context to a unique temp file, pass path as env var
if definition.runs_in_sandbox() {
let ctx_path = format!(
"/tmp/arc-hook-context-{}.json",
std::time::SystemTime::now()
@ -96,7 +120,7 @@ impl HookExecutor for CommandHookExecutor {
env_vars.insert("ARC_HOOK_CONTEXT".to_string(), ctx_path.clone());
}
match sandbox
.exec_command(&command, timeout_ms, None, Some(&env_vars), None)
.exec_command(command, timeout_ms, None, Some(&env_vars), None)
.await
{
Ok(result) => Self::parse_decision(result.exit_code, &result.stdout),
@ -105,23 +129,20 @@ impl HookExecutor for CommandHookExecutor {
},
}
} else {
// Run on host via sh -c
let mut cmd = std::process::Command::new("sh");
cmd.arg("-c").arg(&command);
cmd.arg("-c").arg(command);
if let Some(wd) = work_dir {
cmd.current_dir(wd);
}
for (k, v) in &env_vars {
cmd.env(k, v);
}
// Pipe context JSON to stdin
cmd.stdin(std::process::Stdio::piped());
cmd.stdout(std::process::Stdio::piped());
cmd.stderr(std::process::Stdio::piped());
match cmd.spawn() {
Ok(mut child) => {
// Write context to stdin
if let Some(mut stdin) = child.stdin.take() {
use std::io::Write;
let _ = stdin.write_all(context_json.as_bytes());
@ -141,6 +162,97 @@ impl HookExecutor for CommandHookExecutor {
reason: Some(format!("command spawn failed: {e}")),
},
}
}
}
/// Execute an HTTP hook: POST context JSON and parse the response.
/// Fail-open: non-2xx and connection errors return `Proceed`.
async fn execute_http(
url: &str,
headers: &Option<HashMap<String, String>>,
allowed_env_vars: &[String],
context: &HookContext,
timeout: std::time::Duration,
) -> HookDecision {
let client = reqwest::Client::builder()
.timeout(timeout)
.build()
.unwrap_or_default();
let mut request = client.post(url).json(context);
if let Some(hdrs) = headers {
for (key, value) in hdrs {
let interpolated = interpolate_env_vars(value, allowed_env_vars);
request = request.header(key, interpolated);
}
}
let response = match request.send().await {
Ok(resp) => resp,
Err(e) => {
tracing::warn!(url, error = %e, "HTTP hook request failed, proceeding");
return HookDecision::Proceed;
}
};
if !response.status().is_success() {
tracing::warn!(
url,
status = response.status().as_u16(),
"HTTP hook returned non-2xx, proceeding"
);
return HookDecision::Proceed;
}
let body = match response.text().await {
Ok(text) => text,
Err(e) => {
tracing::warn!(url, error = %e, "HTTP hook body read failed, proceeding");
return HookDecision::Proceed;
}
};
if body.trim().is_empty() {
return HookDecision::Proceed;
}
match serde_json::from_str::<HookDecision>(body.trim()) {
Ok(decision) => decision,
Err(e) => {
tracing::warn!(url, error = %e, "HTTP hook response parse failed, proceeding");
HookDecision::Proceed
}
}
}
}
#[async_trait]
impl HookExecutor for HookExecutorImpl {
async fn execute(
&self,
definition: &HookDefinition,
context: &HookContext,
sandbox: &dyn Sandbox,
work_dir: Option<&Path>,
) -> HookResult {
let start = Instant::now();
let decision = match definition.resolved_hook_type() {
Some(HookType::Command { ref command }) => {
Self::execute_command(definition, command, context, sandbox, work_dir).await
}
Some(HookType::Http {
ref url,
ref headers,
ref allowed_env_vars,
}) => {
Self::execute_http(url, headers, allowed_env_vars, context, definition.timeout())
.await
}
None => HookDecision::Block {
reason: Some("no hook type specified".into()),
},
};
let duration_ms = start.elapsed().as_millis() as u64;
@ -152,6 +264,7 @@ impl HookExecutor for CommandHookExecutor {
}
}
#[cfg(test)]
mod tests {
use super::*;
@ -178,7 +291,7 @@ mod tests {
#[test]
fn parse_decision_exit_0_proceed() {
assert_eq!(
CommandHookExecutor::parse_decision(0, ""),
HookExecutorImpl::parse_decision(0, ""),
HookDecision::Proceed
);
}
@ -187,7 +300,7 @@ mod tests {
fn parse_decision_exit_0_with_json() {
let json = r#"{"decision": "skip", "reason": "not needed"}"#;
assert_eq!(
CommandHookExecutor::parse_decision(0, json),
HookExecutorImpl::parse_decision(0, json),
HookDecision::Skip {
reason: Some("not needed".into())
}
@ -197,7 +310,7 @@ mod tests {
#[test]
fn parse_decision_exit_2_block() {
assert!(matches!(
CommandHookExecutor::parse_decision(2, ""),
HookExecutorImpl::parse_decision(2, ""),
HookDecision::Block { .. }
));
}
@ -206,7 +319,7 @@ mod tests {
fn parse_decision_exit_2_with_json() {
let json = r#"{"decision": "skip", "reason": "skipping"}"#;
assert_eq!(
CommandHookExecutor::parse_decision(2, json),
HookExecutorImpl::parse_decision(2, json),
HookDecision::Skip {
reason: Some("skipping".into())
}
@ -216,7 +329,7 @@ mod tests {
#[test]
fn parse_decision_exit_1_block() {
assert!(matches!(
CommandHookExecutor::parse_decision(1, ""),
HookExecutorImpl::parse_decision(1, ""),
HookDecision::Block { .. }
));
}
@ -225,7 +338,7 @@ mod tests {
fn parse_decision_exit_0_override() {
let json = r#"{"decision": "override", "edge_to": "node_b"}"#;
assert_eq!(
CommandHookExecutor::parse_decision(0, json),
HookExecutorImpl::parse_decision(0, json),
HookDecision::Override {
edge_to: "node_b".into()
}
@ -234,7 +347,7 @@ mod tests {
#[tokio::test]
async fn command_executor_host_success() {
let executor = CommandHookExecutor;
let executor = HookExecutorImpl;
let def = make_definition("exit 0");
let ctx = make_context();
let sandbox = arc_agent::LocalSandbox::new(std::env::current_dir().unwrap());
@ -245,7 +358,7 @@ mod tests {
#[tokio::test]
async fn command_executor_host_failure() {
let executor = CommandHookExecutor;
let executor = HookExecutorImpl;
let def = make_definition("exit 1");
let ctx = make_context();
let sandbox = arc_agent::LocalSandbox::new(std::env::current_dir().unwrap());
@ -255,7 +368,7 @@ mod tests {
#[tokio::test]
async fn command_executor_host_skip_via_exit_2() {
let executor = CommandHookExecutor;
let executor = HookExecutorImpl;
let def = make_definition("exit 2");
let ctx = make_context();
let sandbox = arc_agent::LocalSandbox::new(std::env::current_dir().unwrap());
@ -265,7 +378,7 @@ mod tests {
#[tokio::test]
async fn command_executor_host_json_decision() {
let executor = CommandHookExecutor;
let executor = HookExecutorImpl;
let def =
make_definition(r#"echo '{"decision": "skip", "reason": "test skip"}'"#);
let ctx = make_context();
@ -281,7 +394,7 @@ mod tests {
#[tokio::test]
async fn command_executor_env_vars_set() {
let executor = CommandHookExecutor;
let executor = HookExecutorImpl;
// Print env vars to stdout for verification
let def = make_definition("echo $ARC_EVENT:$ARC_RUN_ID:$ARC_WORKFLOW");
let mut ctx = make_context();
@ -292,16 +405,13 @@ mod tests {
}
#[tokio::test]
async fn command_executor_no_command_blocks() {
let executor = CommandHookExecutor;
async fn no_hook_type_blocks() {
let executor = HookExecutorImpl;
let def = HookDefinition {
name: None,
event: HookEvent::StageStart,
command: None,
hook_type: Some(HookType::Http {
url: "http://example.com".into(),
headers: None,
}),
hook_type: None,
matcher: None,
blocking: None,
timeout_ms: None,
@ -312,4 +422,223 @@ mod tests {
let result = executor.execute(&def, &ctx, &sandbox, None).await;
assert!(matches!(result.decision, HookDecision::Block { .. }));
}
// --- interpolate_env_vars tests ---
#[test]
fn interpolate_resolves_allowed_var() {
std::env::set_var("ARC_TEST_KEY_1", "secret123");
let result = interpolate_env_vars(
"Bearer $ARC_TEST_KEY_1",
&["ARC_TEST_KEY_1".to_string()],
);
assert_eq!(result, "Bearer secret123");
std::env::remove_var("ARC_TEST_KEY_1");
}
#[test]
fn interpolate_resolves_braced_var() {
std::env::set_var("ARC_TEST_KEY_2", "val");
let result = interpolate_env_vars(
"x${ARC_TEST_KEY_2}y",
&["ARC_TEST_KEY_2".to_string()],
);
assert_eq!(result, "xvaly");
std::env::remove_var("ARC_TEST_KEY_2");
}
#[test]
fn interpolate_unlisted_var_becomes_empty() {
std::env::set_var("ARC_TEST_KEY_3", "should_not_appear");
let result = interpolate_env_vars(
"prefix-$ARC_TEST_KEY_3-suffix",
&[],
);
assert_eq!(result, "prefix--suffix");
std::env::remove_var("ARC_TEST_KEY_3");
}
#[test]
fn interpolate_missing_var_becomes_empty() {
std::env::remove_var("ARC_TEST_NOEXIST");
let result = interpolate_env_vars(
"a$ARC_TEST_NOEXIST-b",
&["ARC_TEST_NOEXIST".to_string()],
);
assert_eq!(result, "a-b");
}
#[test]
fn interpolate_no_vars_passes_through() {
assert_eq!(interpolate_env_vars("plain text", &[]), "plain text");
}
#[test]
fn interpolate_mixed_text() {
std::env::set_var("ARC_TEST_A", "hello");
std::env::set_var("ARC_TEST_B", "world");
let result = interpolate_env_vars(
"$ARC_TEST_A ${ARC_TEST_B}!",
&["ARC_TEST_A".to_string(), "ARC_TEST_B".to_string()],
);
assert_eq!(result, "hello world!");
std::env::remove_var("ARC_TEST_A");
std::env::remove_var("ARC_TEST_B");
}
// --- HTTP hook execution tests ---
#[tokio::test]
async fn http_hook_posts_json_and_parses_decision() {
let mut server = mockito::Server::new_async().await;
let mock = server
.mock("POST", "/hook")
.match_header("content-type", "application/json")
.with_status(200)
.with_body(r#"{"decision": "skip", "reason": "not needed"}"#)
.create_async()
.await;
let decision = HookExecutorImpl::execute_http(
&format!("{}/hook", server.url()),
&None,
&[],
&make_context(),
std::time::Duration::from_secs(5),
)
.await;
mock.assert_async().await;
assert_eq!(
decision,
HookDecision::Skip {
reason: Some("not needed".into())
}
);
}
#[tokio::test]
async fn http_hook_empty_2xx_returns_proceed() {
let mut server = mockito::Server::new_async().await;
let mock = server
.mock("POST", "/hook")
.with_status(200)
.with_body("")
.create_async()
.await;
let decision = HookExecutorImpl::execute_http(
&format!("{}/hook", server.url()),
&None,
&[],
&make_context(),
std::time::Duration::from_secs(5),
)
.await;
mock.assert_async().await;
assert_eq!(decision, HookDecision::Proceed);
}
#[tokio::test]
async fn http_hook_non_2xx_returns_proceed() {
let mut server = mockito::Server::new_async().await;
let mock = server
.mock("POST", "/hook")
.with_status(500)
.with_body("Internal Server Error")
.create_async()
.await;
let decision = HookExecutorImpl::execute_http(
&format!("{}/hook", server.url()),
&None,
&[],
&make_context(),
std::time::Duration::from_secs(5),
)
.await;
mock.assert_async().await;
assert_eq!(decision, HookDecision::Proceed);
}
#[tokio::test]
async fn http_hook_connection_failure_returns_proceed() {
let decision = HookExecutorImpl::execute_http(
"http://127.0.0.1:1",
&None,
&[],
&make_context(),
std::time::Duration::from_secs(1),
)
.await;
assert_eq!(decision, HookDecision::Proceed);
}
#[tokio::test]
async fn http_hook_sends_interpolated_headers() {
std::env::set_var("ARC_TEST_TOKEN", "my-secret");
let mut server = mockito::Server::new_async().await;
let mock = server
.mock("POST", "/hook")
.match_header("authorization", "Bearer my-secret")
.with_status(200)
.with_body("")
.create_async()
.await;
let headers = HashMap::from([
("Authorization".to_string(), "Bearer $ARC_TEST_TOKEN".to_string()),
]);
let decision = HookExecutorImpl::execute_http(
&format!("{}/hook", server.url()),
&Some(headers),
&["ARC_TEST_TOKEN".to_string()],
&make_context(),
std::time::Duration::from_secs(5),
)
.await;
mock.assert_async().await;
assert_eq!(decision, HookDecision::Proceed);
std::env::remove_var("ARC_TEST_TOKEN");
}
#[tokio::test]
async fn executor_dispatches_http_hook() {
let mut server = mockito::Server::new_async().await;
let mock = server
.mock("POST", "/hook")
.with_status(200)
.with_body(r#"{"decision": "proceed"}"#)
.create_async()
.await;
let executor = HookExecutorImpl;
let def = HookDefinition {
name: Some("http-test".into()),
event: HookEvent::StageStart,
command: None,
hook_type: Some(HookType::Http {
url: format!("{}/hook", server.url()),
headers: None,
allowed_env_vars: vec![],
}),
matcher: None,
blocking: None,
timeout_ms: Some(5000),
sandbox: Some(false),
};
let ctx = make_context();
let sandbox = arc_agent::LocalSandbox::new(std::env::current_dir().unwrap());
let result = executor.execute(&def, &ctx, &sandbox, None).await;
mock.assert_async().await;
assert_eq!(result.decision, HookDecision::Proceed);
assert_eq!(result.hook_name.as_deref(), Some("http-test"));
}
}

4
crates/arc-workflows/src/hook/runner.rs
View file

@ -5,7 +5,7 @@ use std::sync::Arc;
use arc_agent::Sandbox;
use super::config::{HookConfig, HookDefinition};
use super::executor::{CommandHookExecutor, HookExecutor};
use super::executor::{HookExecutorImpl, HookExecutor};
use super::types::{HookContext, HookDecision};
/// Central orchestrator: filters matching hooks, executes them, merges decisions.
@ -22,7 +22,7 @@ impl HookRunner {
let compiled_matchers = Self::compile_matchers(&config);
Self {
config,
command_executor: Arc::new(CommandHookExecutor),
command_executor: Arc::new(HookExecutorImpl),
compiled_matchers,
}
}