From f425984b608ef9a8983206b9467ad34dc94d73e3 Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:09:34 -0700 Subject: [PATCH] fix(server): authenticate webhook ingress with HMAC + Cedar (ARN-171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inbound webhook route GET|POST /webhooks/{tenant}/{*path} dispatched a spec action with security_ctx: None — no Cedar authorization and no signature verification (Class B, ARN-165). This adds one authenticated ingress: - HMAC-SHA256 over the raw body, constant-time compared (subtle::ConstantTimeEq) against the signature header; secret resolved from the tenant vault via {secret:KEY} templates; fail closed on missing/unresolvable secret. - A restricted webhook:{name} Agent principal authorized through the same authorize_with_context Cedar path as OData writes (default-deny per tenant); denials for authenticated callers recorded via record_authz_denial. Order: lookup(404) -> method(405) -> HMAC(401) -> entity-id(400) -> Cedar(403) -> dispatch. Revives the previously-dead Webhook.hmac_secret/hmac_header spec fields. See docs/adrs/0156-class-b-webhook-ingress.md (incl. rollout note). Red-green exploit tests added (11/11 green): missing/invalid/full-length-wrong signature -> 401, unresolvable secret -> 401, Cedar-denied -> 403, signed+ authorized -> 200. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BM1pdoLhHH7TQgGuiadQiy --- Cargo.lock | 2 + crates/temper-server/Cargo.toml | 2 + crates/temper-server/src/webhooks/receiver.rs | 443 +++++++++-------- .../src/webhooks/receiver_test.rs | 456 ++++++++++++++++++ docs/adrs/0156-class-b-webhook-ingress.md | 227 +++++++++ 5 files changed, 932 insertions(+), 198 deletions(-) create mode 100644 crates/temper-server/src/webhooks/receiver_test.rs create mode 100644 docs/adrs/0156-class-b-webhook-ingress.md diff --git a/Cargo.lock b/Cargo.lock index 5b9d7ed7..ca18b029 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6678,6 +6678,7 @@ dependencies = [ "criterion", "ed25519-dalek", "futures-util", + "hex", "hmac 0.12.1", "hyper 1.8.1", "libc", @@ -6691,6 +6692,7 @@ dependencies = [ "sha1", "sha2 0.10.9", "sqlx", + "subtle", "temper-actor-runtime", "temper-authz", "temper-evolution", diff --git a/crates/temper-server/Cargo.toml b/crates/temper-server/Cargo.toml index 6ce2378d..ae756950 100644 --- a/crates/temper-server/Cargo.toml +++ b/crates/temper-server/Cargo.toml @@ -57,6 +57,8 @@ base64 = "0.22" lru = { workspace = true } sha2 = { workspace = true } hmac = "0.12" +hex = "0.4" +subtle = "2" libc = { workspace = true } # ADR-0055: CPU profile capture. pprof-rs is used because crates.io # `datadog-profiling` is currently an empty placeholder (see ADR for diff --git a/crates/temper-server/src/webhooks/receiver.rs b/crates/temper-server/src/webhooks/receiver.rs index bda79728..74406105 100644 --- a/crates/temper-server/src/webhooks/receiver.rs +++ b/crates/temper-server/src/webhooks/receiver.rs @@ -7,24 +7,42 @@ use std::collections::BTreeMap; +use axum::body::Bytes; use axum::extract::{Path, Query, State}; -use axum::http::{Method, StatusCode}; +use axum::http::{HeaderMap, Method, StatusCode}; use axum::response::IntoResponse; use tracing::instrument; -use crate::request_context::AgentContext; -use crate::state::ServerState; +use temper_authz::{PrincipalKind, SecurityContext}; use temper_runtime::tenant::TenantId; use temper_spec::automaton::Webhook; +use crate::authz::{DenialInput, record_authz_denial}; +use crate::request_context::AgentContext; +use crate::secrets::resolve_secret_templates; +use crate::state::ServerState; + +/// Default header carrying the webhook HMAC signature when the spec's +/// `hmac_header` is unset. +const DEFAULT_SIGNATURE_HEADER: &str = "X-Temper-Signature"; + /// Handle an inbound webhook request. /// /// Route: `GET|POST /webhooks/{tenant}/{*path}` /// /// The handler looks up the webhook configuration from the tenant's spec -/// registry, validates the HTTP method, extracts the entity ID and action -/// parameters, then dispatches the configured action to the target entity. +/// registry, validates the HTTP method, then applies the two gates that guard +/// every other write path (ADR-0156): +/// +/// 1. **Authenticity** — when the webhook declares `hmac_secret`, the request +/// must carry a valid `HMAC-SHA256(secret, raw_body)` signature or it is +/// rejected `401`. +/// 2. **Authorization** — a restricted `webhook:{name}` principal is built and +/// the configured action is authorized through the same Cedar gate as the +/// OData write path; a denied request is rejected `403`. +/// +/// Only after both gates pass is the action dispatched. #[instrument(skip_all, fields( otel.name = %format_args!("{} /webhooks/{}/{}", method, tenant_str, webhook_path), tenant = %tenant_str, @@ -36,6 +54,8 @@ pub async fn handle_webhook( State(state): State, Path((tenant_str, webhook_path)): Path<(String, String)>, Query(query): Query>, + headers: HeaderMap, + body: Bytes, ) -> impl IntoResponse { let tenant = TenantId::new(&tenant_str); @@ -65,6 +85,17 @@ pub async fn handle_webhook( ); } + // Gate 1 — authenticity (ADR-0156). Verify the HMAC signature over the raw + // body before touching entity state. `authenticated` records whether a + // declared secret verified the request; Cedar policies can require it. + let authenticated = match verify_webhook_signature(&state, &tenant, &webhook, &headers, &body) { + Ok(authenticated) => authenticated, + Err((status, message)) => { + tracing::warn!(webhook = %webhook.name, %status, "webhook signature rejected: {message}"); + return (status, message); + } + }; + // Extract entity ID from the configured source. let entity_id = { let param_name = webhook.entity_param.as_deref().unwrap_or("entity_id"); @@ -89,11 +120,68 @@ pub async fn handle_webhook( } let action = &webhook.action; + let webhook_agent_id = format!("webhook:{}", webhook.name); + let security_ctx = webhook_security_context(&webhook.name, authenticated); + + // Gate 2 — authorization (ADR-0156). Authorize the action through the same + // Cedar path as the OData write binding, using the current entity view as + // the resource. A tenant with policies loaded is default-deny, so an + // unpermitted webhook principal is rejected here. + let resource_attrs = match state + .load_authz_resource_snapshot(&tenant, &entity_type, &entity_id) + .await + { + Ok(snapshot) => snapshot.resource_attrs, + Err(_) => minimal_resource_attrs(&entity_id), + }; + + if let Err(denial) = state.authorize_with_context( + &security_ctx, + action, + &entity_type, + &resource_attrs, + tenant.as_str(), + ) { + let reason = denial.to_string(); + tracing::warn!(webhook = %webhook.name, action = %action, "webhook authorization denied: {reason}"); + // Only surface a governance pending-decision for a caller that proved + // it holds the signing secret. An unauthenticated caller (a webhook + // with no declared secret) must not be able to amplify pending-decision + // records by spamming the public route; it gets a plain 403. + if !authenticated { + return (StatusCode::FORBIDDEN, reason); + } + let from_status = resource_attrs + .get("status") + .and_then(serde_json::Value::as_str) + .map(str::to_string); + let decision = record_authz_denial( + &state, + DenialInput { + tenant: tenant.as_str(), + security_ctx: &security_ctx, + agent_id_override: Some(webhook_agent_id.as_str()), + action, + resource_type: &entity_type, + resource_id: &entity_id, + resource_attrs: serde_json::to_value(&resource_attrs).unwrap_or_default(), + reason: &reason, + module_name: None, + from_status, + }, + ) + .await; + return ( + StatusCode::FORBIDDEN, + format!("{reason} (decision: {})", decision.id), + ); + } + let agent_ctx = AgentContext { - security_ctx: None, - agent_id: Some(format!("webhook:{}", webhook.name)), + security_ctx: Some(security_ctx), + agent_id: Some(webhook_agent_id), session_id: None, - agent_type: None, + agent_type: Some("webhook".to_string()), intent: None, ..AgentContext::default() }; @@ -123,6 +211,153 @@ pub async fn handle_webhook( } } +/// Verify the webhook's HMAC-SHA256 signature over the raw request body. +/// +/// Returns: +/// - `Ok(true)` — a declared secret verified the request signature. +/// - `Ok(false)` — the webhook declares no `hmac_secret`; authenticity is left +/// to the Cedar gate (default-deny in production unless explicitly permitted). +/// - `Err((status, message))` — a secret is declared but the request is +/// unsigned, mis-signed, or the secret cannot be resolved. Fail closed: an +/// unverifiable signed webhook must not dispatch. +fn verify_webhook_signature( + state: &ServerState, + tenant: &TenantId, + webhook: &Webhook, + headers: &HeaderMap, + body: &[u8], +) -> Result { + let Some(secret_template) = webhook.hmac_secret.as_deref() else { + return Ok(false); + }; + + let secret = resolve_webhook_secret(state, tenant, secret_template).ok_or_else(|| { + ( + StatusCode::UNAUTHORIZED, + "Webhook signing secret is not configured".to_string(), + ) + })?; + + let header_name = webhook + .hmac_header + .as_deref() + .unwrap_or(DEFAULT_SIGNATURE_HEADER); + let provided = headers + .get(header_name) + .and_then(|value| value.to_str().ok()) + .map(str::trim) + .filter(|value| !value.is_empty()) + .ok_or_else(|| { + ( + StatusCode::UNAUTHORIZED, + format!("Missing webhook signature header '{header_name}'"), + ) + })?; + + if signature_matches(&secret, body, provided) { + Ok(true) + } else { + Err(( + StatusCode::UNAUTHORIZED, + "Webhook signature verification failed".to_string(), + )) + } +} + +/// Resolve the webhook signing secret from the tenant secret store. +/// +/// Supports `{secret:KEY}` templates and literal secrets. Returns `None` (fail +/// closed) when no vault is configured, the template is unresolved, or the +/// resolved value is empty. +fn resolve_webhook_secret( + state: &ServerState, + tenant: &TenantId, + template: &str, +) -> Option { + let vault = state.secrets_vault.as_ref()?; + let mut one = BTreeMap::new(); + one.insert("secret".to_string(), template.to_string()); + let resolved = resolve_secret_templates(&one, vault, tenant.as_str()); + let value = resolved.get("secret")?.clone(); + if value.is_empty() || value.contains("{secret:") { + return None; + } + Some(value) +} + +/// Constant-time comparison of a provided webhook signature against the +/// expected `HMAC-SHA256(secret, body)`. +/// +/// Accepts an optional `sha256=` prefix and is case-insensitive over the hex +/// digest. Uses [`subtle::ConstantTimeEq`] rather than `==` to avoid a timing +/// side channel on the digest. +fn signature_matches(secret: &str, body: &[u8], provided: &str) -> bool { + use hmac::{Hmac, Mac}; + use sha2::Sha256; + use subtle::ConstantTimeEq; + + let Ok(mut mac) = Hmac::::new_from_slice(secret.as_bytes()) else { + return false; + }; + mac.update(body); + let expected_hex = hex::encode(mac.finalize().into_bytes()); + debug_assert_eq!( + expected_hex.len(), + 64, + "HMAC-SHA256 hex digest must be 64 characters" + ); + + // Normalize first (lowercase) so the `sha256=` prefix is matched + // case-insensitively, then strip it. + let normalized = provided.trim().to_ascii_lowercase(); + let provided_hex = normalized + .strip_prefix("sha256=") + .unwrap_or(normalized.as_str()) + .trim(); + + provided_hex + .as_bytes() + .ct_eq(expected_hex.as_bytes()) + .into() +} + +/// Build the restricted Cedar principal for a webhook caller. +/// +/// The principal is an `Agent` named `webhook:{name}` with role/agent_type +/// `webhook`; the `authenticated` attribute records whether an HMAC signature +/// was verified. It is never `System`, so the same tenant Cedar policies that +/// gate the OData write path also gate this route. +fn webhook_security_context(webhook_name: &str, authenticated: bool) -> SecurityContext { + let mut ctx = SecurityContext::from_headers(&[]); + ctx.principal.id = format!("webhook:{webhook_name}"); + ctx.principal.kind = PrincipalKind::Agent; + ctx.principal.role = Some("webhook".to_string()); + ctx.principal.agent_type = Some("webhook".to_string()); + ctx.principal.attributes.insert( + "authenticated".to_string(), + serde_json::Value::Bool(authenticated), + ); + ctx.context_attrs.insert( + "authenticated".to_string(), + serde_json::Value::Bool(authenticated), + ); + ctx.with_action_context(format!("webhook:{webhook_name}")) +} + +/// Minimal Cedar resource view for a webhook target that does not yet exist. +fn minimal_resource_attrs(entity_id: &str) -> BTreeMap { + let mut attrs = BTreeMap::new(); + attrs.insert( + "id".to_string(), + serde_json::Value::String(entity_id.to_string()), + ); + attrs.insert( + "status".to_string(), + serde_json::Value::String(String::new()), + ); + attrs +} + /// Find a webhook matching (tenant, path) in the registry. /// /// Checks the pre-indexed `webhook_routes` map first, then falls back to @@ -158,193 +393,5 @@ fn extract_param(source: &str, query: &BTreeMap) -> Option ServerState { - let csdl = parse_csdl(CSDL_XML).unwrap(); - let system = ActorSystem::new("webhook-test"); - let state = ServerState::new(system, csdl, CSDL_XML.to_string()); - - // Register tenant with webhook-enabled spec. - { - let mut registry = state.registry.write().unwrap(); - let csdl2 = parse_csdl(CSDL_XML).unwrap(); - registry.register_tenant( - "test-tenant", - csdl2, - CSDL_XML.to_string(), - &[("Order", ORDER_IOA_WITH_WEBHOOK)], - ); - } - - state - } - - fn build_test_router() -> axum::Router { - crate::router::build_router(build_test_state()) - } - - #[tokio::test] - async fn webhook_dispatches_action() { - let state = build_test_state(); - let tenant = TenantId::new("test-tenant"); - - // Create entity directly via dispatch. - let _create = state - .get_or_create_tenant_entity( - &tenant, - "Order", - "ent-1", - serde_json::json!({"id": "ent-1"}), - ) - .await - .expect("entity creation should succeed"); - - // Submit to move to "Submitted". - let submit = state - .dispatch_tenant_action( - &tenant, - "Order", - "ent-1", - "SubmitOrder", - serde_json::json!({}), - &AgentContext::default(), - ) - .await - .expect("SubmitOrder should succeed"); - assert!(submit.success, "SubmitOrder should succeed"); - assert_eq!(submit.state.status, "Submitted"); - - // Build router and call webhook. - let app = crate::router::build_router(state); - let response = app - .oneshot( - Request::builder() - .method("GET") - .uri("/webhooks/test-tenant/oauth/callback?state=ent-1&code=abc123") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - - assert_eq!(response.status(), StatusCode::OK); - let body = axum::body::to_bytes(response.into_body(), 1024 * 1024) - .await - .unwrap(); - let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); - assert!( - json["success"].as_bool().unwrap_or(false), - "HandleOAuthCallback should succeed" - ); - assert_eq!(json["state"]["status"], "Authorized"); - } - - #[tokio::test] - async fn webhook_missing_entity_id_returns_400() { - let app = build_test_router(); - - let response = app - .oneshot( - Request::builder() - .method("GET") - .uri("/webhooks/test-tenant/oauth/callback?code=abc123") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - - assert_eq!(response.status(), StatusCode::BAD_REQUEST); - } - - #[tokio::test] - async fn webhook_unknown_path_returns_404() { - let app = build_test_router(); - - let response = app - .oneshot( - Request::builder() - .method("GET") - .uri("/webhooks/test-tenant/nonexistent/path?entity_id=ent-1") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - - assert_eq!(response.status(), StatusCode::NOT_FOUND); - } - - #[tokio::test] - async fn webhook_extracts_query_params() { - let query: BTreeMap = [ - ("code".to_string(), "auth-code-123".to_string()), - ("state".to_string(), "entity-id".to_string()), - ] - .into_iter() - .collect(); - - assert_eq!( - extract_param("query.code", &query), - Some("auth-code-123".to_string()) - ); - assert_eq!( - extract_param("query.state", &query), - Some("entity-id".to_string()) - ); - assert_eq!(extract_param("query.missing", &query), None); - } -} +#[path = "receiver_test.rs"] +mod receiver_test; diff --git a/crates/temper-server/src/webhooks/receiver_test.rs b/crates/temper-server/src/webhooks/receiver_test.rs new file mode 100644 index 00000000..8c4753f6 --- /dev/null +++ b/crates/temper-server/src/webhooks/receiver_test.rs @@ -0,0 +1,456 @@ +use super::*; +use axum::body::Body; +use axum::http::Request; +use temper_runtime::ActorSystem; +use temper_spec::csdl::parse_csdl; +use tower::ServiceExt; + +const CSDL_XML: &str = include_str!("../../../../test-fixtures/specs/model.csdl.xml"); + +/// IOA spec with a webhook declaration for OAuth callback. +const ORDER_IOA_WITH_WEBHOOK: &str = r#" +[automaton] +name = "Order" +states = ["Draft", "Submitted", "Confirmed", "Cancelled", "Authorized"] +initial = "Draft" + +[[action]] +name = "SubmitOrder" +kind = "input" +from = ["Draft"] +to = "Submitted" + +[[action]] +name = "ConfirmOrder" +kind = "input" +from = ["Submitted"] +to = "Confirmed" + +[[action]] +name = "CancelOrder" +kind = "input" +from = ["Draft", "Submitted"] +to = "Cancelled" + +[[action]] +name = "HandleOAuthCallback" +kind = "input" +from = ["Submitted"] +to = "Authorized" +params = ["code"] + +[[webhook]] +name = "oauth_callback" +path = "oauth/callback" +method = "GET" +action = "HandleOAuthCallback" +entity_lookup = "query_param" +entity_param = "state" + +[webhook.extract] +code = "query.code" + +[[webhook]] +name = "pay_callback" +path = "pay/callback" +method = "POST" +action = "HandleOAuthCallback" +entity_lookup = "query_param" +entity_param = "state" +hmac_secret = "{secret:WEBHOOK_SECRET}" +hmac_header = "X-Temper-Signature" + +[webhook.extract] +code = "query.code" + +[[webhook]] +name = "bad_callback" +path = "bad/callback" +method = "POST" +action = "HandleOAuthCallback" +entity_lookup = "query_param" +entity_param = "state" +hmac_secret = "{secret:MISSING_SECRET}" +hmac_header = "X-Temper-Signature" +"#; + +/// Signing secret provisioned into the test tenant's vault for the +/// `pay_callback` signed webhook. +const TEST_WEBHOOK_SECRET: &str = "whsec_test_123"; + +/// Compute a GitHub-style `sha256=` HMAC signature over `body`. +fn sign_webhook(secret: &str, body: &[u8]) -> String { + use hmac::{Hmac, Mac}; + use sha2::Sha256; + let mut mac = + Hmac::::new_from_slice(secret.as_bytes()).expect("HMAC accepts any key length"); + mac.update(body); + format!("sha256={}", hex::encode(mac.finalize().into_bytes())) +} + +fn build_test_state() -> ServerState { + let csdl = parse_csdl(CSDL_XML).unwrap(); + let system = ActorSystem::new("webhook-test"); + let vault = crate::secrets::vault::SecretsVault::new(&[0x11u8; 32]); + vault + .cache_secret( + "test-tenant", + "WEBHOOK_SECRET", + TEST_WEBHOOK_SECRET.to_string(), + ) + .expect("cache webhook secret"); + let state = ServerState::new(system, csdl, CSDL_XML.to_string()).with_secrets_vault(vault); + + // Register tenant with webhook-enabled spec. + { + let mut registry = state.registry.write().unwrap(); + let csdl2 = parse_csdl(CSDL_XML).unwrap(); + registry.register_tenant( + "test-tenant", + csdl2, + CSDL_XML.to_string(), + &[("Order", ORDER_IOA_WITH_WEBHOOK)], + ); + } + + state +} + +/// Create `ent-1` and move it to `Submitted` so the webhook's +/// `HandleOAuthCallback` (from `Submitted`) is a valid transition. +async fn seed_submitted_order(state: &ServerState) { + let tenant = TenantId::new("test-tenant"); + state + .get_or_create_tenant_entity( + &tenant, + "Order", + "ent-1", + serde_json::json!({"id": "ent-1"}), + ) + .await + .expect("entity creation should succeed"); + state + .dispatch_tenant_action( + &tenant, + "Order", + "ent-1", + "SubmitOrder", + serde_json::json!({}), + &AgentContext::default(), + ) + .await + .expect("SubmitOrder should succeed"); +} + +fn build_test_router() -> axum::Router { + crate::router::build_router(build_test_state()) +} + +#[tokio::test] +async fn webhook_dispatches_action() { + let state = build_test_state(); + let tenant = TenantId::new("test-tenant"); + + // Create entity directly via dispatch. + let _create = state + .get_or_create_tenant_entity( + &tenant, + "Order", + "ent-1", + serde_json::json!({"id": "ent-1"}), + ) + .await + .expect("entity creation should succeed"); + + // Submit to move to "Submitted". + let submit = state + .dispatch_tenant_action( + &tenant, + "Order", + "ent-1", + "SubmitOrder", + serde_json::json!({}), + &AgentContext::default(), + ) + .await + .expect("SubmitOrder should succeed"); + assert!(submit.success, "SubmitOrder should succeed"); + assert_eq!(submit.state.status, "Submitted"); + + // Build router and call webhook. + let app = crate::router::build_router(state); + let response = app + .oneshot( + Request::builder() + .method("GET") + .uri("/webhooks/test-tenant/oauth/callback?state=ent-1&code=abc123") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = axum::body::to_bytes(response.into_body(), 1024 * 1024) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert!( + json["success"].as_bool().unwrap_or(false), + "HandleOAuthCallback should succeed" + ); + assert_eq!(json["state"]["status"], "Authorized"); +} + +#[tokio::test] +async fn webhook_missing_entity_id_returns_400() { + let app = build_test_router(); + + let response = app + .oneshot( + Request::builder() + .method("GET") + .uri("/webhooks/test-tenant/oauth/callback?code=abc123") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); +} + +#[tokio::test] +async fn webhook_unknown_path_returns_404() { + let app = build_test_router(); + + let response = app + .oneshot( + Request::builder() + .method("GET") + .uri("/webhooks/test-tenant/nonexistent/path?entity_id=ent-1") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); +} + +#[tokio::test] +async fn webhook_extracts_query_params() { + let query: BTreeMap = [ + ("code".to_string(), "auth-code-123".to_string()), + ("state".to_string(), "entity-id".to_string()), + ] + .into_iter() + .collect(); + + assert_eq!( + extract_param("query.code", &query), + Some("auth-code-123".to_string()) + ); + assert_eq!( + extract_param("query.state", &query), + Some("entity-id".to_string()) + ); + assert_eq!(extract_param("query.missing", &query), None); +} + +// ── Class B: authenticated webhook ingress (ARN-171) ────────────────────── + +/// Exploit test: a POST to a signed webhook with NO signature header must +/// be rejected. Before ARN-171 this dispatched the action unauthenticated. +#[tokio::test] +async fn webhook_rejects_missing_signature() { + let app = crate::router::build_router(build_test_state()); + let body = br#"{"event":"paid"}"#; + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/webhooks/test-tenant/pay/callback?state=ent-1&code=abc123") + .body(Body::from(body.to_vec())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +/// Exploit test: a POST to a signed webhook with a WRONG signature must be +/// rejected. Before ARN-171 the signature was never computed or compared. +#[tokio::test] +async fn webhook_rejects_invalid_signature() { + let app = crate::router::build_router(build_test_state()); + let body = br#"{"event":"paid"}"#; + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/webhooks/test-tenant/pay/callback?state=ent-1&code=abc123") + .header("X-Temper-Signature", "sha256=deadbeefdeadbeef") + .body(Body::from(body.to_vec())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +/// A correctly-signed request whose webhook principal the tenant's Cedar +/// policy does NOT permit must be denied (403). Before ARN-171 the webhook +/// path never touched Cedar, so a denied principal succeeded. +#[tokio::test] +async fn webhook_valid_signature_but_cedar_denies_returns_403() { + let state = build_test_state(); + seed_submitted_order(&state).await; + // Tenant policy set that permits a DIFFERENT action — switches the + // tenant to its own (default-deny) policy set, so the webhook action + // (HandleOAuthCallback) is not permitted. + state + .authz + .reload_tenant_policies( + "test-tenant", + r#"permit(principal, action == Action::"SubmitOrder", resource is Order);"#, + ) + .expect("install Cedar policy"); + + let body = br#"{"event":"paid"}"#; + let sig = sign_webhook(TEST_WEBHOOK_SECRET, body); + let app = crate::router::build_router(state); + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/webhooks/test-tenant/pay/callback?state=ent-1&code=abc123") + .header("X-Temper-Signature", sig) + .body(Body::from(body.to_vec())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); +} + +/// A correctly-signed AND Cedar-authorized request still dispatches the +/// action and transitions the entity. +#[tokio::test] +async fn webhook_valid_signature_and_authorized_succeeds() { + let state = build_test_state(); + seed_submitted_order(&state).await; + state + .authz + .reload_tenant_policies( + "test-tenant", + r#"permit(principal, action == Action::"HandleOAuthCallback", resource is Order);"#, + ) + .expect("install Cedar policy"); + + let body = br#"{"event":"paid"}"#; + let sig = sign_webhook(TEST_WEBHOOK_SECRET, body); + let app = crate::router::build_router(state); + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/webhooks/test-tenant/pay/callback?state=ent-1&code=abc123") + .header("X-Temper-Signature", sig) + .body(Body::from(body.to_vec())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let bytes = axum::body::to_bytes(response.into_body(), 1024 * 1024) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(json["state"]["status"], "Authorized"); +} + +/// A well-formed but WRONG 64-hex signature must be rejected. Unlike the +/// short `deadbeef` case (rejected on length), this exercises the +/// equal-length constant-time byte comparison. +#[tokio::test] +async fn webhook_rejects_full_length_wrong_signature() { + let app = crate::router::build_router(build_test_state()); + let body = br#"{"event":"paid"}"#; + let wrong = format!("sha256={}", "a".repeat(64)); + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/webhooks/test-tenant/pay/callback?state=ent-1&code=abc123") + .header("X-Temper-Signature", wrong) + .body(Body::from(body.to_vec())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +/// A webhook that declares `hmac_secret` referencing an unresolvable +/// `{secret:...}` must fail closed (401) — the signing secret is not +/// configured, so no request can be authenticated. +#[tokio::test] +async fn webhook_unresolvable_secret_returns_401() { + let app = crate::router::build_router(build_test_state()); + let body = br#"{"event":"paid"}"#; + // Any signature — the secret can't be resolved, so it must be rejected + // before comparison. + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/webhooks/test-tenant/bad/callback?state=ent-1&code=abc123") + .header("X-Temper-Signature", "sha256=whatever") + .body(Body::from(body.to_vec())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +/// A webhook with NO declared secret is still governed by Cedar: in a +/// tenant with a deny-by-default policy set that does not permit the +/// webhook action, the (unauthenticated) call is denied 403 — proving the +/// authorization gate runs independently of the signature gate. +#[tokio::test] +async fn webhook_no_secret_cedar_denied_returns_403() { + let state = build_test_state(); + seed_submitted_order(&state).await; + // Policy set permits a different action → default-deny for the + // webhook's HandleOAuthCallback. + state + .authz + .reload_tenant_policies( + "test-tenant", + r#"permit(principal, action == Action::"SubmitOrder", resource is Order);"#, + ) + .expect("install Cedar policy"); + + // oauth_callback (GET) declares no hmac_secret — no signature required. + let app = crate::router::build_router(state); + let response = app + .oneshot( + Request::builder() + .method("GET") + .uri("/webhooks/test-tenant/oauth/callback?state=ent-1&code=abc123") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); +} diff --git a/docs/adrs/0156-class-b-webhook-ingress.md b/docs/adrs/0156-class-b-webhook-ingress.md new file mode 100644 index 00000000..e6dde76b --- /dev/null +++ b/docs/adrs/0156-class-b-webhook-ingress.md @@ -0,0 +1,227 @@ +# ADR-0156: Authenticated webhook ingress (Class B) + +- Status: Proposed +- Date: 2026-07-06 +- Deciders: Temper core maintainers +- Related: + - ARN-171 (kernel Class B), ARN-165 (remediation epic), ARN-168 (TemperPaw counterpart) + - `crates/temper-server/src/webhooks/receiver.rs` (the inbound handler) + - `crates/temper-server/src/odata/authz.rs`, `crates/temper-server/src/odata/bindings.rs` (the OData write path this mirrors) + - `crates/temper-spec/src/automaton/types.rs` (`Webhook` spec: `hmac_secret`, `hmac_header`) + +## Context + +The inbound webhook route `GET|POST /webhooks/{tenant}/{*path}` +(`webhooks/receiver.rs`) dispatches a spec-declared entity action via +`dispatch_tenant_action(...)` with `security_ctx: None`. Two independent +protections that guard every other write path are absent here: + +1. **No authorization.** `dispatch_tenant_action_core` has no Cedar gate; + Cedar is enforced only in the OData binding/extractor layers, which this + route never touches. A principal Cedar would deny on the OData path + succeeds through the webhook path. +2. **No authenticity.** There is no signature check. The `Webhook` spec has + carried `hmac_secret` and `hmac_header` fields the whole time, but + `receiver.rs` never read them — the fields were dead. + +Tenant is taken from the URL path and `entity_id` from a client query +param — both attacker-chosen. So `POST /webhooks//pay/callback?entity_id=INV-123` +drives an arbitrary declared action on an arbitrary entity, unauthenticated. + +This is the kernel instance of the systemic **Class B** finding in ARN-165. +The TemperPaw side (ARN-168, a no-op HMAC that only checks header presence) +mirrors this design once landed. + +## Decision + +Make the webhook route a single authenticated ingress that applies **both** +protections before any dispatch, reusing the exact primitives the OData +write path already uses. No new dispatch path, no bespoke auth. + +### Sub-Decision 1: Cedar gate on every webhook, always + +Before dispatch, build a real `SecurityContext` for the webhook caller and +call `state.authorize_with_context(&ctx, action, entity_type, &attrs, tenant)` +— the same function the OData bound-action path calls. Resource attributes +come from `load_authz_resource_snapshot` (the same loader the OData and +trigger paths use), falling back to a minimal `{id, status}` view when the +target entity does not yet exist. + +The webhook principal is **restricted, not `System`**: + +- `kind = Agent`, `id = "webhook:{name}"`, `role = "webhook"`, + `agent_type = "webhook"`, +- attribute `authenticated: bool` = whether an HMAC signature was verified, +- `action_context = "webhook:{name}"` (ADR-0040 provenance). + +**Why this approach**: Cedar is already default-deny per tenant in +production (a tenant with a loaded policy set denies anything not explicitly +permitted — see `AuthzEngine::authorize_for_tenant`). Attaching a real, +narrow principal means the webhook route is governed by the same policies as +every other write, and is **fail-closed by default**: a webhook only +dispatches if the tenant's policy explicitly permits the `webhook:*` +principal for that action. Policies can additionally require +`principal.authenticated == true` to insist on a verified signature. + +Denials for an **authenticated** caller (one that proved it holds the signing +secret) are recorded via `record_authz_denial` exactly like the OData path, so +a denied webhook intent surfaces as a pending decision rather than failing +silently. Denials for an unauthenticated caller (a webhook with no declared +secret) return a plain `403` and are **not** recorded — otherwise anyone could +amplify pending-decision/governance records by spamming the public route. + +### Sub-Decision 2: HMAC-SHA256 authenticity when a secret is declared + +When the webhook declares `hmac_secret`, the request must carry a valid +signature or it is rejected with `401` before dispatch: + +- Resolve the signing secret from the **tenant secret store** via the + existing `{secret:KEY}` template resolver (`resolve_secret_templates` over + `state.secrets_vault`). A literal secret is also accepted. +- Read the signature from `hmac_header` (default `X-Temper-Signature`). A + missing header is a `401`. +- Compute `HMAC-SHA256(secret, raw_request_body)`, hex-encode, and compare + against the provided value (optional `sha256=` prefix, case-insensitive) + using a **constant-time comparison** (`subtle::ConstantTimeEq`) — never + `==` on the digest. +- **Fail closed on misconfiguration**: if the secret is declared but cannot + be resolved (no vault, or an unresolved `{secret:KEY}`), reject with `401` + and log — an unverifiable webhook must not dispatch. + +The raw body is read as `axum::body::Bytes`, which is bounded by axum's +default `DefaultBodyLimit` (2 MiB) so a webhook body cannot buffer unbounded. + +**Why this approach**: HMAC over the raw body is the de-facto standard for +signed webhooks (GitHub `X-Hub-Signature-256`, Stripe, Datadog). Reusing the +spec's existing `hmac_secret`/`hmac_header` fields and the existing secret +vault means no new configuration surface. Constant-time comparison prevents a +timing side channel on the digest. + +### Sub-Decision 3: Order of checks + +`lookup (404)` → `method (405)` → `HMAC verify (401, only if a secret is +declared)` → `entity-id extraction (400)` → `Cedar gate (403)` → `dispatch`. +HMAC runs before entity resolution so an unauthenticated caller learns +nothing about entity state. + +### Tenant/principal are authenticated, not merely claimed + +The tenant still appears in the URL, but it is no longer *trusted* from the +URL: the signing secret is read from **that tenant's** vault, so only a +caller holding the tenant's secret can produce a valid signature. The +principal is derived server-side (`webhook:{name}`), never from an +attacker-supplied header. + +## Rollout Plan — backward compatibility (read before deploy) + +This change makes the webhook route Cedar-authorized **always**. For a tenant +with a policy set loaded (default-deny), a webhook that works today +unauthenticated will start returning `403` unless a policy permits its +`webhook:{name}` principal for the dispatched action. The webhook principal is +`Agent`-kind with `role == "webhook"` and `agent_type == "webhook"`. + +**Required permit shape** (per webhook, added to the tenant's Cedar policy set): + +```cedar +permit( + principal is Agent, + action == Action::"", + resource is +) when { principal.role == "webhook" }; +``` + +Tighten further with `&& principal.authenticated == true` to require a verified +HMAC signature, or match a specific webhook via +`principal.action_context == "webhook:"`. + +**Inbound webhooks that exist in THIS repo** (grep `[[webhook]]` in entity +specs — only entity-spec webhooks hit `/webhooks/{tenant}/{*path}`; the +`url`-based `[[webhook]]` blocks in `reference-apps/*/integration.toml` are the +**outbound** dispatcher and are unaffected): + +- `test-fixtures/specs/gmail_oauth.ioa.toml` — `oauth_callback` → action + `OAuthCallback` on the OAuth entity. Test fixture only; its DST test drives + `sim.step("OAuthCallback", …)` directly, not the HTTP route, so it does not + regress. A real deploy of this spec would need: + `permit(principal is Agent, action == Action::"OAuthCallback", resource is GmailConnection) when { principal.role == "webhook" };` + +**Out-of-repo production webhooks** (kernel cannot see these; flagged for the +deploy owner): any app deployed on Temper that uses an inbound kernel webhook — +notably **TemperPaw OAuth callbacks** and any **Katagami** integration +callbacks — must ship the matching `webhook:*` permit **in the same deploy** as +this kernel change, or those callbacks will 403. The platform's per-spec Cedar +generator (`temper-platform/hooks/generate_cedar.rs`) does **not** currently +emit a webhook permit automatically — it generates `ThisAgent`-scoped policies +from GovernanceDecision fields — so the permit must be added explicitly (via +the app's policy set) until a generator rule is added (follow-up). + +**Rollout order**: land the permit policies for live webhooks first (or in the +same change set), then deploy this kernel gate. A policy-less tenant is +unaffected (permissive fallback), but no production app tenant is policy-less. + +## Consequences + +### Positive +- The webhook route is governed by the same Cedar policies as every other + write, and authenticated by HMAC when a secret is configured. No code path + dispatches with `security_ctx: None`. +- The long-dead `hmac_secret`/`hmac_header` spec fields become live. +- Establishes the kernel pattern the TemperPaw fix (ARN-168) mirrors. + +**Precondition on the Cedar fail-closed guarantee.** `authorize_for_tenant` +is default-deny only for a tenant that has a **policy set loaded**; a tenant +with zero policies falls through to the engine's global fallback, which in the +default `ServerState` is `AuthzEngine::permissive()` (permit-all). The platform +generates a per-tenant Cedar policy set on spec registration +(`temper-platform` `hooks/generate_cedar.rs` → `reload_tenant_policies`), so a +deployed app tenant is default-deny; a policy-less tenant is permit-all. This +is exactly the OData write path's behavior — the webhook route is now at +parity, not stricter. For a **no-secret** webhook this means: it is fully +governed (fail-closed) once the tenant has a deny-by-default policy set, which +is a deployment requirement, not an automatic property of this route. + +### Negative / Tradeoffs +- A webhook with **no** declared `hmac_secret` is authenticated only by its + Cedar policy (plus any unguessable capability in its payload, e.g. an + OAuth `state` nonce). This deliberately preserves the OAuth-callback + capability, which providers cannot HMAC-sign. Any webhook whose caller + *can* sign should declare a secret. This is the central tradeoff; the + stricter alternative (below) was rejected to avoid breaking OAuth. +- Webhook authors must add a Cedar permit for the `webhook:*` principal for + webhooks to fire in production — intentional, and auditable. + +### Risks +- Signature-format assumptions: hex digest with an optional `sha256=` + prefix. Providers using base64 or a timestamped scheme (Stripe's `t=,v1=`) + are out of scope here (see Non-Goals) and would need a follow-up encoding + selector on the spec. + +### DST Compliance +- `webhooks/receiver.rs` is an HTTP boundary, not part of the deterministic + actor simulation. No wall-clock, RNG, or ambient I/O is introduced in the + new code: HMAC/SHA are pure CPU; the `SecurityContext` correlation UUID is + minted inside `temper-authz` as before; secrets come from the injected + vault. `BTreeMap` is used throughout. No new `// determinism-ok` needed. + +## Non-Goals +- Base64 or provider-specific signature schemes (Stripe timestamped, Shopify + base64). Hex (optionally `sha256=`-prefixed) only. +- Replay protection (timestamp/nonce windows) — a possible follow-up. +- Changing the outbound `WebhookDispatcher`. + +## Alternatives Considered + +1. **Hard-require HMAC on every webhook (reject any webhook with no + declared secret).** Strongest authenticity, and closest to the epic's + "fail closed when no credential source is configured" wording. Rejected + because it breaks OAuth-style callbacks, which the provider redirects + without a signature — a working capability we must preserve. Cedar + default-deny already provides the fail-closed guarantee for the no-secret + case. +2. **Cedar gate only, no HMAC.** Closes the authorization bypass but leaves + signed webhooks (payments/events) spoofable by anyone a policy permits. + Insufficient — the issue calls for authenticity too. +3. **A dedicated `Webhook` `PrincipalKind`.** Cleaner typing, but a + cross-crate Cedar-schema change for little gain; `Agent` + `role/ + agent_type = "webhook"` + `action_context` expresses the same policy + surface today.