From 721224159da229460e5750b2598cee003fecce60 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 5 Mar 2026 02:06:19 -0500 Subject: [PATCH] Add TLS mode config for HTTP hooks Introduce a `tls` field on HTTP hooks with three modes: `verify` (default, requires https + cert validation), `no_verify` (requires https, skips cert validation), and `off` (allows http, skips cert validation). This prevents hooks from accidentally sending credentials over plaintext connections. Co-Authored-By: Claude Opus 4.6 --- crates/arc-workflows/src/hook/config.rs | 66 ++++++++++++++++++ crates/arc-workflows/src/hook/executor.rs | 84 ++++++++++++++++++++++- crates/arc-workflows/src/hook/mod.rs | 2 +- 3 files changed, 149 insertions(+), 3 deletions(-) diff --git a/crates/arc-workflows/src/hook/config.rs b/crates/arc-workflows/src/hook/config.rs index 8b704cff1..aab13baca 100644 --- a/crates/arc-workflows/src/hook/config.rs +++ b/crates/arc-workflows/src/hook/config.rs @@ -2,6 +2,19 @@ use serde::Deserialize; use super::types::HookEvent; +/// TLS verification mode for HTTP hooks. +#[derive(Debug, Clone, Copy, Deserialize, PartialEq, Eq, Default)] +#[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)] #[serde(tag = "type", rename_all = "snake_case")] @@ -12,6 +25,8 @@ pub enum HookType { headers: Option>, #[serde(default)] allowed_env_vars: Vec, + #[serde(default)] + tls: TlsMode, }, } @@ -194,6 +209,7 @@ Authorization = "Bearer $API_KEY" url, headers, allowed_env_vars, + .. } => { assert_eq!(url, "https://hooks.example.com/start"); assert_eq!(allowed_env_vars, vec!["API_KEY", "SECRET"]); @@ -407,6 +423,56 @@ sandbox = false assert_eq!(merged.hooks[0].event, HookEvent::RunComplete); } + #[test] + fn parse_http_hook_tls_defaults_to_verify() { + 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 { tls, .. } => assert_eq!(tls, TlsMode::Verify), + _ => panic!("expected Http hook type"), + } + } + + #[test] + fn parse_http_hook_tls_no_verify() { + let toml = r#" +[[hooks]] +event = "run_complete" +type = "http" +url = "https://hooks.example.com/done" +tls = "no_verify" +"#; + let config: HookConfig = toml::from_str(toml).unwrap(); + let hook = &config.hooks[0]; + match hook.resolved_hook_type().unwrap() { + HookType::Http { tls, .. } => assert_eq!(tls, TlsMode::NoVerify), + _ => panic!("expected Http hook type"), + } + } + + #[test] + fn parse_http_hook_tls_off() { + let toml = r#" +[[hooks]] +event = "run_complete" +type = "http" +url = "http://localhost:8080/done" +tls = "off" +"#; + let config: HookConfig = toml::from_str(toml).unwrap(); + let hook = &config.hooks[0]; + match hook.resolved_hook_type().unwrap() { + HookType::Http { tls, .. } => assert_eq!(tls, TlsMode::Off), + _ => panic!("expected Http hook type"), + } + } + #[test] fn parse_multiple_hooks() { let toml = r#" diff --git a/crates/arc-workflows/src/hook/executor.rs b/crates/arc-workflows/src/hook/executor.rs index 25d41de9b..d61acc0df 100644 --- a/crates/arc-workflows/src/hook/executor.rs +++ b/crates/arc-workflows/src/hook/executor.rs @@ -6,7 +6,7 @@ use async_trait::async_trait; use arc_agent::Sandbox; -use super::config::{HookDefinition, HookType}; +use super::config::{HookDefinition, HookType, TlsMode}; use super::types::{HookContext, HookDecision, HookResult}; /// Trait for executing hooks via different transports. @@ -171,11 +171,28 @@ impl HookExecutorImpl { url: &str, headers: &Option>, allowed_env_vars: &[String], + tls: &TlsMode, context: &HookContext, timeout: std::time::Duration, ) -> HookDecision { + // Enforce URL scheme based on TLS mode + match tls { + TlsMode::Verify | TlsMode::NoVerify => { + if !url.starts_with("https://") { + return HookDecision::Block { + reason: Some(format!( + "HTTP hook URL must use https:// (tls mode is {tls:?})" + )), + }; + } + } + TlsMode::Off => {} + } + + let accept_invalid = matches!(tls, TlsMode::NoVerify | TlsMode::Off); let client = reqwest::Client::builder() .timeout(timeout) + .danger_accept_invalid_certs(accept_invalid) .build() .unwrap_or_default(); @@ -246,8 +263,9 @@ impl HookExecutor for HookExecutorImpl { ref url, ref headers, ref allowed_env_vars, + ref tls, }) => { - Self::execute_http(url, headers, allowed_env_vars, context, definition.timeout()) + Self::execute_http(url, headers, allowed_env_vars, tls, context, definition.timeout()) .await } None => HookDecision::Block { @@ -503,6 +521,7 @@ mod tests { &format!("{}/hook", server.url()), &None, &[], + &TlsMode::Off, &make_context(), std::time::Duration::from_secs(5), ) @@ -531,6 +550,7 @@ mod tests { &format!("{}/hook", server.url()), &None, &[], + &TlsMode::Off, &make_context(), std::time::Duration::from_secs(5), ) @@ -554,6 +574,7 @@ mod tests { &format!("{}/hook", server.url()), &None, &[], + &TlsMode::Off, &make_context(), std::time::Duration::from_secs(5), ) @@ -569,6 +590,7 @@ mod tests { "http://127.0.0.1:1", &None, &[], + &TlsMode::Off, &make_context(), std::time::Duration::from_secs(1), ) @@ -598,6 +620,7 @@ mod tests { &format!("{}/hook", server.url()), &Some(headers), &["ARC_TEST_TOKEN".to_string()], + &TlsMode::Off, &make_context(), std::time::Duration::from_secs(5), ) @@ -608,6 +631,62 @@ mod tests { std::env::remove_var("ARC_TEST_TOKEN"); } + // --- TLS mode enforcement tests --- + + #[tokio::test] + async fn http_hook_rejects_http_url_when_tls_verify() { + let decision = HookExecutorImpl::execute_http( + "http://example.com/hook", + &None, + &[], + &TlsMode::Verify, + &make_context(), + std::time::Duration::from_secs(5), + ) + .await; + + assert!(matches!(decision, HookDecision::Block { .. })); + } + + #[tokio::test] + async fn http_hook_rejects_http_url_when_tls_no_verify() { + let decision = HookExecutorImpl::execute_http( + "http://example.com/hook", + &None, + &[], + &TlsMode::NoVerify, + &make_context(), + std::time::Duration::from_secs(5), + ) + .await; + + assert!(matches!(decision, HookDecision::Block { .. })); + } + + #[tokio::test] + async fn http_hook_allows_http_url_when_tls_off() { + 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, + &[], + &TlsMode::Off, + &make_context(), + std::time::Duration::from_secs(5), + ) + .await; + + mock.assert_async().await; + assert_eq!(decision, HookDecision::Proceed); + } + #[tokio::test] async fn executor_dispatches_http_hook() { let mut server = mockito::Server::new_async().await; @@ -627,6 +706,7 @@ mod tests { url: format!("{}/hook", server.url()), headers: None, allowed_env_vars: vec![], + tls: TlsMode::Off, }), matcher: None, blocking: None, diff --git a/crates/arc-workflows/src/hook/mod.rs b/crates/arc-workflows/src/hook/mod.rs index 3a347b5c7..4fad81372 100644 --- a/crates/arc-workflows/src/hook/mod.rs +++ b/crates/arc-workflows/src/hook/mod.rs @@ -3,6 +3,6 @@ pub mod executor; pub mod runner; pub mod types; -pub use config::{HookConfig, HookDefinition, HookType}; +pub use config::{HookConfig, HookDefinition, HookType, TlsMode}; pub use runner::HookRunner; pub use types::{HookContext, HookDecision, HookEvent};