Skip to content

Add URL validation to operational server webhooks #1887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion server/svix-server/src/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ use figment::{
use ipnet::IpNet;
use serde::{Deserialize, Deserializer};
use tracing::Level;
use validator::{Validate, ValidationError};
use url::Url;
use validator::{Validate, ValidationError, ValidationErrors};

use crate::{
core::{cryptography::Encryption, security::JwtSigningConfig},
error::Result,
v1::utils::validation_error,
};

fn deserialize_main_secret<'de, D>(deserializer: D) -> Result<Encryption, D::Error>
Expand Down Expand Up @@ -74,6 +76,37 @@ fn default_redis_pending_duration_secs() -> u64 {
45
}

fn validate_operational_webhook_url(url: &Option<String>) -> Result<(), ValidationError> {
if let Some(url_str) = url {
match Url::parse(url_str) {
Ok(url) => {
// Verify scheme is http or https
if url.scheme() != "http" && url.scheme() != "https" {
return Err(validation_error(
Some("operational_webhook_address"),
Some("URL scheme must be http or https"),
));
}

// Verify there's a host
if url.host().is_none() {
return Err(validation_error(
Some("operational_webhook_address"),
Some("URL must include a valid host"),
));
}
}
Err(_) => {
return Err(validation_error(
Some("operational_webhook_address"),
Some("Invalid URL format"),
));
}
}
}
Ok(())
}

#[derive(Clone, Debug, Deserialize, Validate)]
#[validate(
schema(function = "validate_config_complete"),
Expand All @@ -85,6 +118,7 @@ pub struct ConfigurationInner {

/// The address to send operational webhooks to. When None, operational webhooks will not be
/// sent. When Some, the API server with the given URL will be used to send operational webhooks.
#[validate(custom = "validate_operational_webhook_url")]
pub operational_webhook_address: Option<String>,

/// The main secret used by Svix. Used for client-side encryption of sensitive data, etc.
Expand Down
50 changes: 44 additions & 6 deletions server/svix-server/src/core/operational_webhooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,25 @@ pub struct OperationalWebhookSenderInner {

impl OperationalWebhookSenderInner {
pub fn new(keys: Arc<JwtSigningConfig>, url: Option<String>) -> Arc<Self> {
// Sanitize the URL if present
let sanitized_url = url.as_ref().map(|url_str| {
// Remove trailing slashes
let mut cleaned_url = url_str.trim().to_string();
while cleaned_url.ends_with('/') {
cleaned_url.pop();
}

// Return the cleaned URL or original if empty
if !cleaned_url.is_empty() {
cleaned_url
} else {
url_str.clone()
}
});

Arc::new(Self {
signing_config: keys,
url,
url: sanitized_url,
})
}

Expand Down Expand Up @@ -151,6 +167,7 @@ impl OperationalWebhookSenderInner {
.to_string();

let recipient_org_id = recipient_org_id.to_string();
let url_clone = url.clone(); // Clone for use in the async block

tokio::spawn(async move {
// This sends a webhook under the Svix management organization. This organization contains
Expand All @@ -171,20 +188,41 @@ impl OperationalWebhookSenderInner {

match resp {
Ok(_) => {}
// Ignore 404s because not every org will have an associated application
// Handle 404s with more context
Err(svix::error::Error::Http(svix::error::HttpErrorContent {
status: StatusCode::NOT_FOUND,
..
})) => {
tracing::warn!(
"Operational webhooks are enabled but no listener set for {}",
recipient_org_id,
// Try to determine if it's a connection issue or an app not found issue
if let Some(app_resp) = svix_api.application().get(recipient_org_id.clone(), None).await.ok() {
// App exists but endpoint not found
tracing::warn!(
"Operational webhooks are enabled, but no endpoint is configured for organization {} (app exists)",
recipient_org_id,
);
} else {
// App doesn't exist
tracing::warn!(
"Operational webhooks are enabled, but no application exists for organization {}",
recipient_org_id,
);
}
}
// Add specific handling for connection errors
Err(svix::error::Error::Http(svix::error::HttpErrorContent {
status: StatusCode::BAD_REQUEST,
..
})) => {
tracing::error!(
"Failed sending operational webhook: Bad request. Check URL format: {}",
url_clone,
);
}
Err(e) => {
tracing::error!(
"Failed sending operational webhook for {} {}",
"Failed sending operational webhook for {} to URL {}: {}",
recipient_org_id,
url_clone,
e.to_string()
);
}
Expand Down
Loading