From 55e8a5cff1f185b1dbd332d37b877972efa1ed7d Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 2 Apr 2026 20:55:23 +0000 Subject: [PATCH] fix: add HMAC signature verification to Slack interactive callback endpoint (#8611) Co-authored-by: Claude Opus 4.6 (1M context) --- backend/windmill-api/src/oauth2_oss.rs | 15 +++++++-- backend/windmill-api/src/slack_approvals.rs | 35 +++++++++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/backend/windmill-api/src/oauth2_oss.rs b/backend/windmill-api/src/oauth2_oss.rs index 2b47c66bdf..fc54ef1ba9 100644 --- a/backend/windmill-api/src/oauth2_oss.rs +++ b/backend/windmill-api/src/oauth2_oss.rs @@ -159,13 +159,24 @@ pub async fn check_nb_of_user(db: &DB) -> error::Result<()> { #[derive(Clone, Debug)] #[cfg(not(feature = "private"))] pub struct SlackVerifier { - _mac: HmacSha256, + mac: HmacSha256, } #[cfg(not(feature = "private"))] impl SlackVerifier { pub fn new>(secret: S) -> anyhow::Result { HmacSha256::new_from_slice(secret.as_ref()) - .map(|mac| SlackVerifier { _mac: mac }) + .map(|mac| SlackVerifier { mac }) .map_err(|_| anyhow::anyhow!("invalid secret")) } + + pub fn verify(&self, ts: &str, body: &str, exp_sig: &str) -> anyhow::Result<()> { + let basestring = format!("v0:{}:{}", ts, body); + let mut mac = self.mac.clone(); + mac.update(basestring.as_bytes()); + let sig = format!("v0={}", hex::encode(mac.finalize().into_bytes())); + if sig != exp_sig { + Err(anyhow::anyhow!("signature mismatch"))?; + } + Ok(()) + } } diff --git a/backend/windmill-api/src/slack_approvals.rs b/backend/windmill-api/src/slack_approvals.rs index 34a79b4651..28e327c1ca 100644 --- a/backend/windmill-api/src/slack_approvals.rs +++ b/backend/windmill-api/src/slack_approvals.rs @@ -1,7 +1,9 @@ use axum::{ - extract::{Form, Path, Query}, + extract::{Path, Query}, Extension, }; +use bytes::Bytes; +use http::HeaderMap; use hyper::StatusCode; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -119,12 +121,41 @@ struct PrivateMetadata { hide_cancel: Option, } +#[cfg(feature = "oauth2")] +fn verify_slack_callback_signature(headers: &HeaderMap, body: &str) -> Result<(), Error> { + if let Some(sv) = crate::SLACK_SIGNING_SECRET.as_ref() { + let sig = headers + .get("X-Slack-Signature") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let ts = headers + .get("X-Slack-Request-Timestamp") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + sv.verify(ts, body, sig) + .map_err(|_| Error::BadRequest("Slack signature verification failed".to_string()))?; + } + Ok(()) +} + +#[cfg(not(feature = "oauth2"))] +fn verify_slack_callback_signature(_headers: &HeaderMap, _body: &str) -> Result<(), Error> { + Ok(()) +} + pub async fn slack_app_callback_handler( authed: Option, opt_tokened: OptTokened, Extension(db): Extension, - Form(form_data): Form, + headers: HeaderMap, + body: Bytes, ) -> Result { + let body_str = String::from_utf8_lossy(&body); + verify_slack_callback_signature(&headers, &body_str)?; + + let form_data: SlackFormData = serde_urlencoded::from_bytes(&body) + .map_err(|e| Error::BadRequest(format!("invalid form data: {}", e)))?; + tracing::debug!("Form data: {:#?}", form_data); let payload: Payload = serde_json::from_str(&form_data.payload)?; tracing::debug!("Payload: {:#?}", payload);