mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix: require Feishu webhook authentication (#1638)
* fix: require Feishu webhook authentication * fix: handle Feishu v2 webhook token auth * fix: skip empty verification token write, consistent with app_id/app_secret Address zmanian review nit #4: only write verification_token to workspace when present, matching the if-let pattern used for app_id and app_secret. Functionally identical (the auth check filters empty strings), but consistent. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
7234700c78
commit
30db07c58e
Generated
+7
@@ -44,6 +44,7 @@ version = "0.1.0"
|
|||||||
dependencies = [
|
dependencies = [
|
||||||
"serde",
|
"serde",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
|
"subtle",
|
||||||
"wit-bindgen",
|
"wit-bindgen",
|
||||||
]
|
]
|
||||||
|
|
||||||
@@ -208,6 +209,12 @@ dependencies = [
|
|||||||
"smallvec",
|
"smallvec",
|
||||||
]
|
]
|
||||||
|
|
||||||
|
[[package]]
|
||||||
|
name = "subtle"
|
||||||
|
version = "2.6.1"
|
||||||
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
|
checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292"
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "syn"
|
name = "syn"
|
||||||
version = "2.0.117"
|
version = "2.0.117"
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ wit-bindgen = "0.36"
|
|||||||
# Serialization
|
# Serialization
|
||||||
serde = { version = "1.0", features = ["derive"] }
|
serde = { version = "1.0", features = ["derive"] }
|
||||||
serde_json = "1.0"
|
serde_json = "1.0"
|
||||||
|
subtle = "2.6"
|
||||||
|
|
||||||
# Exclude from parent workspace (this is a standalone WASM component)
|
# Exclude from parent workspace (this is a standalone WASM component)
|
||||||
|
|
||||||
|
|||||||
@@ -27,7 +27,7 @@
|
|||||||
{
|
{
|
||||||
"name": "feishu_verification_token",
|
"name": "feishu_verification_token",
|
||||||
"prompt": "Enter your Feishu/Lark Verification Token (from Event Subscription webhook settings)",
|
"prompt": "Enter your Feishu/Lark Verification Token (from Event Subscription webhook settings)",
|
||||||
"optional": true
|
"optional": false
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
"setup_url": "https://open.feishu.cn/app"
|
"setup_url": "https://open.feishu.cn/app"
|
||||||
@@ -63,13 +63,15 @@
|
|||||||
},
|
},
|
||||||
"webhook": {
|
"webhook": {
|
||||||
"secret_header": "X-Feishu-Verification-Token",
|
"secret_header": "X-Feishu-Verification-Token",
|
||||||
"secret_name": "feishu_verification_token"
|
"secret_name": "feishu_verification_token",
|
||||||
|
"managed_by_host": false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"config": {
|
"config": {
|
||||||
"app_id": null,
|
"app_id": null,
|
||||||
"app_secret": null,
|
"app_secret": null,
|
||||||
|
"verification_token": null,
|
||||||
"api_base": "https://open.feishu.cn",
|
"api_base": "https://open.feishu.cn",
|
||||||
"owner_id": null,
|
"owner_id": null,
|
||||||
"dm_policy": "pairing",
|
"dm_policy": "pairing",
|
||||||
|
|||||||
@@ -23,7 +23,8 @@
|
|||||||
//! - App credentials (app_id, app_secret) are injected by the host into
|
//! - App credentials (app_id, app_secret) are injected by the host into
|
||||||
//! the config JSON during startup for token exchange
|
//! the config JSON during startup for token exchange
|
||||||
//! - Bearer token for API calls is obtained via token exchange and cached
|
//! - Bearer token for API calls is obtained via token exchange and cached
|
||||||
//! - Verification token validated by host for webhook requests
|
//! - Webhook requests must be authenticated by the host or by a matching
|
||||||
|
//! Feishu verification token in the request body
|
||||||
|
|
||||||
// Generate bindings from the WIT file
|
// Generate bindings from the WIT file
|
||||||
wit_bindgen::generate!({
|
wit_bindgen::generate!({
|
||||||
@@ -32,6 +33,7 @@ wit_bindgen::generate!({
|
|||||||
});
|
});
|
||||||
|
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
use subtle::ConstantTimeEq;
|
||||||
|
|
||||||
// Re-export generated types
|
// Re-export generated types
|
||||||
use exports::near::agent::channel::{
|
use exports::near::agent::channel::{
|
||||||
@@ -50,6 +52,7 @@ const ALLOW_FROM_PATH: &str = "allow_from";
|
|||||||
const API_BASE_PATH: &str = "api_base";
|
const API_BASE_PATH: &str = "api_base";
|
||||||
const APP_ID_PATH: &str = "app_id";
|
const APP_ID_PATH: &str = "app_id";
|
||||||
const APP_SECRET_PATH: &str = "app_secret";
|
const APP_SECRET_PATH: &str = "app_secret";
|
||||||
|
const VERIFICATION_TOKEN_PATH: &str = "verification_token";
|
||||||
const TOKEN_PATH: &str = "tenant_access_token";
|
const TOKEN_PATH: &str = "tenant_access_token";
|
||||||
const TOKEN_EXPIRY_PATH: &str = "token_expiry";
|
const TOKEN_EXPIRY_PATH: &str = "token_expiry";
|
||||||
|
|
||||||
@@ -102,6 +105,10 @@ struct FeishuEventHeader {
|
|||||||
/// Tenant key.
|
/// Tenant key.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
tenant_key: Option<String>,
|
tenant_key: Option<String>,
|
||||||
|
|
||||||
|
/// Verification token for v2 event payloads.
|
||||||
|
#[serde(default)]
|
||||||
|
token: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Message receive event payload (im.message.receive_v1).
|
/// Message receive event payload (im.message.receive_v1).
|
||||||
@@ -251,6 +258,9 @@ struct FeishuConfig {
|
|||||||
/// Feishu App Secret (for token exchange).
|
/// Feishu App Secret (for token exchange).
|
||||||
app_secret: Option<String>,
|
app_secret: Option<String>,
|
||||||
|
|
||||||
|
/// Feishu Event Subscription verification token.
|
||||||
|
verification_token: Option<String>,
|
||||||
|
|
||||||
/// API base URL. Defaults to "https://open.feishu.cn" (use
|
/// API base URL. Defaults to "https://open.feishu.cn" (use
|
||||||
/// "https://open.larksuite.com" for Lark international).
|
/// "https://open.larksuite.com" for Lark international).
|
||||||
#[serde(default = "default_api_base")]
|
#[serde(default = "default_api_base")]
|
||||||
@@ -300,6 +310,9 @@ impl Guest for FeishuChannel {
|
|||||||
if let Some(ref app_secret) = config.app_secret {
|
if let Some(ref app_secret) = config.app_secret {
|
||||||
let _ = channel_host::workspace_write(APP_SECRET_PATH, app_secret);
|
let _ = channel_host::workspace_write(APP_SECRET_PATH, app_secret);
|
||||||
}
|
}
|
||||||
|
if let Some(ref verification_token) = config.verification_token {
|
||||||
|
let _ = channel_host::workspace_write(VERIFICATION_TOKEN_PATH, verification_token);
|
||||||
|
}
|
||||||
|
|
||||||
if let Some(owner_id) = &config.owner_id {
|
if let Some(owner_id) = &config.owner_id {
|
||||||
let _ = channel_host::workspace_write(OWNER_ID_PATH, owner_id);
|
let _ = channel_host::workspace_write(OWNER_ID_PATH, owner_id);
|
||||||
@@ -376,6 +389,23 @@ impl Guest for FeishuChannel {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
let configured_token =
|
||||||
|
channel_host::workspace_read(VERIFICATION_TOKEN_PATH).filter(|token| !token.is_empty());
|
||||||
|
if !is_authenticated_webhook(
|
||||||
|
req.secret_validated,
|
||||||
|
configured_token.as_deref(),
|
||||||
|
request_verification_token(&event),
|
||||||
|
) {
|
||||||
|
channel_host::log(
|
||||||
|
channel_host::LogLevel::Warn,
|
||||||
|
"Rejecting unauthenticated Feishu webhook request",
|
||||||
|
);
|
||||||
|
return json_response(
|
||||||
|
401,
|
||||||
|
serde_json::json!({"error": "Webhook authentication failed"}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// Handle URL verification challenge (initial webhook setup).
|
// Handle URL verification challenge (initial webhook setup).
|
||||||
if event.event_type.as_deref() == Some("url_verification") {
|
if event.event_type.as_deref() == Some("url_verification") {
|
||||||
if let Some(challenge) = &event.challenge {
|
if let Some(challenge) = &event.challenge {
|
||||||
@@ -839,6 +869,31 @@ fn json_response(status: u16, body: serde_json::Value) -> OutgoingHttpResponse {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_authenticated_webhook(
|
||||||
|
secret_validated: bool,
|
||||||
|
configured_token: Option<&str>,
|
||||||
|
request_token: Option<&str>,
|
||||||
|
) -> bool {
|
||||||
|
if secret_validated {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
match (configured_token, request_token) {
|
||||||
|
(Some(expected), Some(provided)) => {
|
||||||
|
bool::from(expected.as_bytes().ct_eq(provided.as_bytes()))
|
||||||
|
}
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn request_verification_token(event: &FeishuEvent) -> Option<&str> {
|
||||||
|
event
|
||||||
|
.header
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|header| header.token.as_deref())
|
||||||
|
.or(event.token.as_deref())
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -862,7 +917,10 @@ mod tests {
|
|||||||
fn parse_token_response_rejects_missing_token() {
|
fn parse_token_response_rejects_missing_token() {
|
||||||
let json = r#"{"code": 0, "msg": "ok", "expire": 7200}"#;
|
let json = r#"{"code": 0, "msg": "ok", "expire": 7200}"#;
|
||||||
let result: Result<TenantAccessTokenResponse, _> = serde_json::from_str(json);
|
let result: Result<TenantAccessTokenResponse, _> = serde_json::from_str(json);
|
||||||
assert!(result.is_err(), "should fail when tenant_access_token is missing");
|
assert!(
|
||||||
|
result.is_err(),
|
||||||
|
"should fail when tenant_access_token is missing"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -894,4 +952,64 @@ mod tests {
|
|||||||
assert_eq!(resp.code, 10003);
|
assert_eq!(resp.code, 10003);
|
||||||
assert!(resp.tenant_access_token.is_empty());
|
assert!(resp.tenant_access_token.is_empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn webhook_auth_requires_host_auth_or_matching_verification_token() {
|
||||||
|
assert!(
|
||||||
|
!is_authenticated_webhook(false, None, Some("token")),
|
||||||
|
"requests without any configured verification mechanism must be rejected"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!is_authenticated_webhook(false, Some("expected"), None),
|
||||||
|
"requests missing the Feishu token must be rejected when host auth did not pass"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!is_authenticated_webhook(false, Some("expected"), Some("wrong")),
|
||||||
|
"requests with the wrong Feishu token must be rejected"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
is_authenticated_webhook(false, Some("expected"), Some("expected")),
|
||||||
|
"matching Feishu verification token should authenticate the request"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
is_authenticated_webhook(true, None, None),
|
||||||
|
"host-authenticated requests should still be accepted"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
is_authenticated_webhook(true, Some("expected"), Some("wrong")),
|
||||||
|
"host authentication should take precedence over body token checks"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn request_verification_token_prefers_v2_header_token() {
|
||||||
|
let event: FeishuEvent = serde_json::from_str(
|
||||||
|
r#"{
|
||||||
|
"schema": "2.0",
|
||||||
|
"header": {
|
||||||
|
"event_id": "evt_123",
|
||||||
|
"event_type": "im.message.receive_v1",
|
||||||
|
"token": "header-token"
|
||||||
|
},
|
||||||
|
"event": {}
|
||||||
|
}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(request_verification_token(&event), Some("header-token"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn request_verification_token_falls_back_to_top_level_token() {
|
||||||
|
let event: FeishuEvent = serde_json::from_str(
|
||||||
|
r#"{
|
||||||
|
"type": "url_verification",
|
||||||
|
"challenge": "abc",
|
||||||
|
"token": "top-level-token"
|
||||||
|
}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(request_verification_token(&event), Some("top-level-token"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -317,6 +317,14 @@ impl LoadedChannel {
|
|||||||
.map(|f| f.webhook_secret_name())
|
.map(|f| f.webhook_secret_name())
|
||||||
.unwrap_or_else(|| format!("{}_webhook_secret", self.channel.channel_name()))
|
.unwrap_or_else(|| format!("{}_webhook_secret", self.channel.channel_name()))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether the host should enforce generic webhook-secret validation.
|
||||||
|
pub fn webhook_secret_managed_by_host(&self) -> bool {
|
||||||
|
self.capabilities_file
|
||||||
|
.as_ref()
|
||||||
|
.map(|f| f.webhook_secret_managed_by_host())
|
||||||
|
.unwrap_or(true)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Results from loading multiple channels.
|
/// Results from loading multiple channels.
|
||||||
|
|||||||
@@ -185,6 +185,19 @@ impl ChannelCapabilitiesFile {
|
|||||||
.and_then(|w| w.secret_name.clone())
|
.and_then(|w| w.secret_name.clone())
|
||||||
.unwrap_or_else(|| format!("{}_webhook_secret", self.name))
|
.unwrap_or_else(|| format!("{}_webhook_secret", self.name))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether the host should enforce generic webhook-secret validation.
|
||||||
|
///
|
||||||
|
/// Defaults to true. Channels can opt out when they validate the shared
|
||||||
|
/// secret themselves using provider-specific request body fields.
|
||||||
|
pub fn webhook_secret_managed_by_host(&self) -> bool {
|
||||||
|
self.capabilities
|
||||||
|
.channel
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|c| c.webhook.as_ref())
|
||||||
|
.and_then(|w| w.managed_by_host)
|
||||||
|
.unwrap_or(true)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Schema for channel capabilities.
|
/// Schema for channel capabilities.
|
||||||
@@ -302,6 +315,14 @@ pub struct WebhookSchema {
|
|||||||
/// Secret name in secrets store for HMAC-SHA256 signing (Slack-style).
|
/// Secret name in secrets store for HMAC-SHA256 signing (Slack-style).
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub hmac_secret_name: Option<String>,
|
pub hmac_secret_name: Option<String>,
|
||||||
|
|
||||||
|
/// Whether the host/router should enforce generic webhook-secret
|
||||||
|
/// validation before the channel sees the request.
|
||||||
|
///
|
||||||
|
/// Default: true. Set to false when the provider sends the shared secret
|
||||||
|
/// in a provider-specific request field rather than the configured header.
|
||||||
|
#[serde(default)]
|
||||||
|
pub managed_by_host: Option<bool>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Setup configuration schema.
|
/// Setup configuration schema.
|
||||||
@@ -611,6 +632,25 @@ mod tests {
|
|||||||
Some("X-Telegram-Bot-Api-Secret-Token")
|
Some("X-Telegram-Bot-Api-Secret-Token")
|
||||||
);
|
);
|
||||||
assert_eq!(file.webhook_secret_name(), "telegram_webhook_secret");
|
assert_eq!(file.webhook_secret_name(), "telegram_webhook_secret");
|
||||||
|
assert!(file.webhook_secret_managed_by_host());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_webhook_schema_can_disable_host_managed_secret_validation() {
|
||||||
|
let json = r#"{
|
||||||
|
"name": "feishu",
|
||||||
|
"capabilities": {
|
||||||
|
"channel": {
|
||||||
|
"webhook": {
|
||||||
|
"secret_name": "feishu_verification_token",
|
||||||
|
"managed_by_host": false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}"#;
|
||||||
|
|
||||||
|
let file = ChannelCapabilitiesFile::from_json(json).unwrap();
|
||||||
|
assert!(!file.webhook_secret_managed_by_host());
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -139,13 +139,18 @@ async fn register_channel(
|
|||||||
};
|
};
|
||||||
|
|
||||||
let secret_header = loaded.webhook_secret_header().map(|s| s.to_string());
|
let secret_header = loaded.webhook_secret_header().map(|s| s.to_string());
|
||||||
|
let host_webhook_secret = if loaded.webhook_secret_managed_by_host() {
|
||||||
|
webhook_secret.clone()
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
|
||||||
let webhook_path = format!("/webhook/{}", channel_name);
|
let webhook_path = format!("/webhook/{}", channel_name);
|
||||||
let endpoints = vec![RegisteredEndpoint {
|
let endpoints = vec![RegisteredEndpoint {
|
||||||
channel_name: channel_name.clone(),
|
channel_name: channel_name.clone(),
|
||||||
path: webhook_path,
|
path: webhook_path,
|
||||||
methods: vec!["POST".to_string()],
|
methods: vec!["POST".to_string()],
|
||||||
require_secret: webhook_secret.is_some(),
|
require_secret: host_webhook_secret.is_some(),
|
||||||
}];
|
}];
|
||||||
|
|
||||||
let channel_arc = Arc::new(loaded.channel.with_owner_actor_id(owner_actor_id.clone()));
|
let channel_arc = Arc::new(loaded.channel.with_owner_actor_id(owner_actor_id.clone()));
|
||||||
@@ -205,7 +210,7 @@ async fn register_channel(
|
|||||||
|
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
channel = %channel_name,
|
channel = %channel_name,
|
||||||
has_webhook_secret = webhook_secret.is_some(),
|
has_webhook_secret = host_webhook_secret.is_some(),
|
||||||
secret_header = ?secret_header,
|
secret_header = ?secret_header,
|
||||||
"Registering channel with router"
|
"Registering channel with router"
|
||||||
);
|
);
|
||||||
@@ -214,7 +219,7 @@ async fn register_channel(
|
|||||||
.register(
|
.register(
|
||||||
Arc::clone(&channel_arc),
|
Arc::clone(&channel_arc),
|
||||||
endpoints,
|
endpoints,
|
||||||
webhook_secret.clone(),
|
host_webhook_secret.clone(),
|
||||||
secret_header,
|
secret_header,
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
@@ -392,8 +397,9 @@ pub async fn inject_channel_credentials(
|
|||||||
/// placeholders in URLs and headers, so this function fills config fields
|
/// placeholders in URLs and headers, so this function fills config fields
|
||||||
/// that map to secret names.
|
/// that map to secret names.
|
||||||
///
|
///
|
||||||
/// Mapping: for a channel named "feishu", secrets `feishu_app_id` and
|
/// Mapping: for a channel named "feishu", secrets `feishu_app_id`,
|
||||||
/// `feishu_app_secret` are injected as config keys `app_id` and `app_secret`.
|
/// `feishu_app_secret`, and `feishu_verification_token` are injected as config
|
||||||
|
/// keys `app_id`, `app_secret`, and `verification_token`.
|
||||||
async fn inject_channel_secrets_into_config(
|
async fn inject_channel_secrets_into_config(
|
||||||
channel_name: &str,
|
channel_name: &str,
|
||||||
secrets_store: &Option<Arc<dyn SecretsStore + Send + Sync>>,
|
secrets_store: &Option<Arc<dyn SecretsStore + Send + Sync>>,
|
||||||
@@ -404,6 +410,7 @@ async fn inject_channel_secrets_into_config(
|
|||||||
"feishu" => &[
|
"feishu" => &[
|
||||||
("app_id", "feishu_app_id"),
|
("app_id", "feishu_app_id"),
|
||||||
("app_secret", "feishu_app_secret"),
|
("app_secret", "feishu_app_secret"),
|
||||||
|
("verification_token", "feishu_verification_token"),
|
||||||
],
|
],
|
||||||
_ => return,
|
_ => return,
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user