From 323cf97e33c2bf80fd03f338acd89bb11a4216f6 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Wed, 15 Apr 2026 01:21:51 -0400 Subject: [PATCH 1/3] feat(gateway): eliminate the Sentry ingestion gap Signed-off-by: Yordis Prieto --- devops/docker/compose/.env.example | 7 + .../compose/services/trogon-gateway/README.md | 8 + rsworkspace/Cargo.lock | 20 + rsworkspace/Cargo.toml | 1 + rsworkspace/crates/trogon-gateway/Cargo.toml | 1 + .../crates/trogon-gateway/src/config.rs | 260 ++++++++++ rsworkspace/crates/trogon-gateway/src/http.rs | 11 + .../crates/trogon-gateway/src/streams.rs | 14 +- .../crates/trogon-source-sentry/Cargo.toml | 25 + .../crates/trogon-source-sentry/src/config.rs | 12 + .../trogon-source-sentry/src/constants.rs | 13 + .../crates/trogon-source-sentry/src/lib.rs | 10 + .../src/sentry_client_secret.rs | 39 ++ .../crates/trogon-source-sentry/src/server.rs | 476 ++++++++++++++++++ .../trogon-source-sentry/src/signature.rs | 104 ++++ 15 files changed, 999 insertions(+), 2 deletions(-) create mode 100644 rsworkspace/crates/trogon-source-sentry/Cargo.toml create mode 100644 rsworkspace/crates/trogon-source-sentry/src/config.rs create mode 100644 rsworkspace/crates/trogon-source-sentry/src/constants.rs create mode 100644 rsworkspace/crates/trogon-source-sentry/src/lib.rs create mode 100644 rsworkspace/crates/trogon-source-sentry/src/sentry_client_secret.rs create mode 100644 rsworkspace/crates/trogon-source-sentry/src/server.rs create mode 100644 rsworkspace/crates/trogon-source-sentry/src/signature.rs diff --git a/devops/docker/compose/.env.example b/devops/docker/compose/.env.example index 7664ceebf..38f9d9565 100644 --- a/devops/docker/compose/.env.example +++ b/devops/docker/compose/.env.example @@ -41,6 +41,13 @@ # TROGON_SOURCE_NOTION_STREAM_MAX_AGE_SECS=604800 # TROGON_SOURCE_NOTION_NATS_ACK_TIMEOUT_SECS=10 +# --- Sentry Source --- +# TROGON_SOURCE_SENTRY_CLIENT_SECRET= +# TROGON_SOURCE_SENTRY_SUBJECT_PREFIX=sentry +# TROGON_SOURCE_SENTRY_STREAM_NAME=SENTRY +# TROGON_SOURCE_SENTRY_STREAM_MAX_AGE_SECS=604800 +# TROGON_SOURCE_SENTRY_NATS_ACK_TIMEOUT_SECS=10 + # --- Discord Source --- # TROGON_SOURCE_DISCORD_BOT_TOKEN= # TROGON_SOURCE_DISCORD_GATEWAY_INTENTS=guilds,guild_members,guild_messages,guild_message_reactions,direct_messages,message_content,guild_voice_states diff --git a/devops/docker/compose/services/trogon-gateway/README.md b/devops/docker/compose/services/trogon-gateway/README.md index 192d9e478..d48a724fc 100644 --- a/devops/docker/compose/services/trogon-gateway/README.md +++ b/devops/docker/compose/services/trogon-gateway/README.md @@ -16,6 +16,7 @@ prefix: | incident.io | `/incidentio/webhook` | `TROGON_SOURCE_INCIDENTIO_SIGNING_SECRET` | | Linear | `/linear/webhook` | `TROGON_SOURCE_LINEAR_WEBHOOK_SECRET` | | Notion | `/notion/webhook` | `TROGON_SOURCE_NOTION_VERIFICATION_TOKEN` | +| Sentry | `/sentry/webhook` | `TROGON_SOURCE_SENTRY_CLIENT_SECRET` | The gateway port is configured via `TROGON_GATEWAY_PORT` (default `8080`). Liveness and readiness probes are available at `GET /-/liveness` and `GET /-/readiness`. @@ -67,6 +68,13 @@ then point the Notion webhook endpoint at `/notion/webhook`. Verified events are forwarded to NATS on `{subject_prefix}.{type}` subjects such as `notion.page.created`. +## Sentry webhooks + +Sentry integration-platform webhooks sign the raw JSON body with the app client +secret. Configure `TROGON_SOURCE_SENTRY_CLIENT_SECRET`, point the webhook URL +at `/sentry/webhook`, and the gateway will forward verified payloads to NATS on +`{subject_prefix}.{resource}.{action}` subjects such as `sentry.issue.created`. + ## Exposing webhooks with ngrok ```bash diff --git a/rsworkspace/Cargo.lock b/rsworkspace/Cargo.lock index 147c3687c..ce7d2ce32 100644 --- a/rsworkspace/Cargo.lock +++ b/rsworkspace/Cargo.lock @@ -3469,6 +3469,7 @@ dependencies = [ "trogon-source-incidentio", "trogon-source-linear", "trogon-source-notion", + "trogon-source-sentry", "trogon-source-slack", "trogon-source-telegram", "trogon-std", @@ -3621,6 +3622,25 @@ dependencies = [ "trogon-std", ] +[[package]] +name = "trogon-source-sentry" +version = "0.1.0" +dependencies = [ + "async-nats", + "axum", + "hex", + "hmac", + "serde", + "serde_json", + "sha2", + "tokio", + "tower", + "tracing", + "tracing-subscriber", + "trogon-nats", + "trogon-std", +] + [[package]] name = "trogon-source-slack" version = "0.1.0" diff --git a/rsworkspace/Cargo.toml b/rsworkspace/Cargo.toml index 08c5f0507..91b6db179 100644 --- a/rsworkspace/Cargo.toml +++ b/rsworkspace/Cargo.toml @@ -21,6 +21,7 @@ trogon-source-gitlab = { path = "crates/trogon-source-gitlab" } trogon-source-incidentio = { path = "crates/trogon-source-incidentio" } trogon-source-linear = { path = "crates/trogon-source-linear" } trogon-source-notion = { path = "crates/trogon-source-notion" } +trogon-source-sentry = { path = "crates/trogon-source-sentry" } trogon-source-slack = { path = "crates/trogon-source-slack" } trogon-source-telegram = { path = "crates/trogon-source-telegram" } trogon-std = { path = "crates/trogon-std" } diff --git a/rsworkspace/crates/trogon-gateway/Cargo.toml b/rsworkspace/crates/trogon-gateway/Cargo.toml index 86a3b32af..9b4e3047c 100644 --- a/rsworkspace/crates/trogon-gateway/Cargo.toml +++ b/rsworkspace/crates/trogon-gateway/Cargo.toml @@ -27,6 +27,7 @@ trogon-source-gitlab = { workspace = true } trogon-source-incidentio = { workspace = true } trogon-source-linear = { workspace = true } trogon-source-notion = { workspace = true } +trogon-source-sentry = { workspace = true } trogon-source-slack = { workspace = true } trogon-source-telegram = { workspace = true } trogon-std = { workspace = true, features = ["clap", "telemetry-http"] } diff --git a/rsworkspace/crates/trogon-gateway/src/config.rs b/rsworkspace/crates/trogon-gateway/src/config.rs index d5d29ea89..2ff39c01a 100644 --- a/rsworkspace/crates/trogon-gateway/src/config.rs +++ b/rsworkspace/crates/trogon-gateway/src/config.rs @@ -15,6 +15,7 @@ use trogon_source_incidentio::config::IncidentioConfig as IncidentioSourceConfig use trogon_source_incidentio::incidentio_signing_secret::IncidentioSigningSecret; use trogon_source_linear::config::LinearWebhookSecret; use trogon_source_notion::NotionVerificationToken; +use trogon_source_sentry::SentryClientSecret; use trogon_source_slack::config::SlackSigningSecret; use trogon_source_telegram::config::TelegramWebhookSecret; use trogon_std::{NonZeroDuration, ZeroDuration}; @@ -39,6 +40,10 @@ pub enum ConfigValidationError { field: &'static str, error: Box, }, + MissingField { + source: &'static str, + field: &'static str, + }, InvalidSubjectToken { source: &'static str, field: &'static str, @@ -58,6 +63,10 @@ impl ConfigValidationError { } } + fn missing(source: &'static str, field: &'static str) -> Self { + Self::MissingField { source, field } + } + fn invalid_subject_token( source: &'static str, field: &'static str, @@ -90,6 +99,7 @@ impl fmt::Display for ConfigValidationError { write!(f, "{source}: invalid {field}: {error}") } } + Self::MissingField { source, field } => write!(f, "{source}: missing {field}"), Self::InvalidSubjectToken { source, field, @@ -103,6 +113,7 @@ impl std::error::Error for ConfigValidationError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { Self::InvalidField { error, .. } => Some(error.as_ref()), + Self::MissingField { .. } => None, Self::InvalidSubjectToken { .. } => None, } } @@ -169,6 +180,8 @@ struct SourcesConfig { linear: LinearConfig, #[config(nested)] notion: NotionConfig, + #[config(nested)] + sentry: SentryConfig, } #[derive(Config)] @@ -316,6 +329,22 @@ struct NotionConfig { nats_ack_timeout_secs: u64, } +#[derive(Config)] +struct SentryConfig { + #[config(env = "TROGON_SOURCE_SENTRY_STATUS")] + status: Option, + #[config(env = "TROGON_SOURCE_SENTRY_CLIENT_SECRET")] + client_secret: Option, + #[config(env = "TROGON_SOURCE_SENTRY_SUBJECT_PREFIX", default = "sentry")] + subject_prefix: String, + #[config(env = "TROGON_SOURCE_SENTRY_STREAM_NAME", default = "SENTRY")] + stream_name: String, + #[config(env = "TROGON_SOURCE_SENTRY_STREAM_MAX_AGE_SECS", default = 604_800)] + stream_max_age_secs: u64, + #[config(env = "TROGON_SOURCE_SENTRY_NATS_ACK_TIMEOUT_SECS", default = 10)] + nats_ack_timeout_secs: u64, +} + pub struct ResolvedHttpServerConfig { pub port: u16, } @@ -332,6 +361,7 @@ pub struct ResolvedConfig { pub incidentio: Option, pub linear: Option, pub notion: Option, + pub sentry: Option, } impl ResolvedConfig { @@ -344,6 +374,7 @@ impl ResolvedConfig { || self.incidentio.is_some() || self.linear.is_some() || self.notion.is_some() + || self.sentry.is_some() } } @@ -372,6 +403,7 @@ fn resolve(cfg: GatewayConfig, nats_overrides: &NatsArgs) -> Result v, @@ -403,6 +435,7 @@ fn resolve(cfg: GatewayConfig, nats_overrides: &NatsArgs) -> Result, +) -> Option { + let explicitly_enabled = matches!( + section.status.as_deref(), + Some(status) if status.trim().eq_ignore_ascii_case("enabled") + ); + + if !resolve_source_status("sentry", section.status.as_deref(), errors) { + return None; + } + + let client_secret = match section.client_secret { + Some(secret) => match SentryClientSecret::new(secret) { + Ok(secret) => secret, + Err(error) => { + errors.push(ConfigValidationError::invalid( + "sentry", + "client_secret", + error, + )); + return None; + } + }, + None => { + if explicitly_enabled { + errors.push(ConfigValidationError::missing("sentry", "client_secret")); + } + return None; + } + }; + + let subject_prefix = match NatsToken::new(section.subject_prefix) { + Ok(token) => token, + Err(error) => { + errors.push(ConfigValidationError::invalid_subject_token( + "sentry", + "subject_prefix", + error, + )); + return None; + } + }; + + let stream_name = match NatsToken::new(section.stream_name) { + Ok(token) => token, + Err(error) => { + errors.push(ConfigValidationError::invalid_subject_token( + "sentry", + "stream_name", + error, + )); + return None; + } + }; + + let nats_ack_timeout = match NonZeroDuration::from_secs(section.nats_ack_timeout_secs) { + Ok(duration) => duration, + Err(error) => { + errors.push(ConfigValidationError::invalid( + "sentry", + "nats_ack_timeout_secs", + error, + )); + return None; + } + }; + + let stream_max_age = match StreamMaxAge::from_secs(section.stream_max_age_secs) { + Ok(age) => age, + Err(error) => { + errors.push(ConfigValidationError::invalid( + "sentry", + "stream_max_age_secs", + error, + )); + return None; + } + }; + + Some(trogon_source_sentry::SentryConfig { + client_secret, + subject_prefix, + stream_name, + stream_max_age, + nats_ack_timeout, + }) +} + fn resolve_source_status( source: &'static str, status: Option<&str>, @@ -1185,6 +1308,15 @@ verification_token = "{token}" ) } + fn sentry_toml(secret: &str) -> String { + format!( + r#" +[sources.sentry] +client_secret = "{secret}" +"# + ) + } + fn incidentio_valid_test_secret() -> String { ["whsec_", "dGVzdC1zZWNyZXQ="].concat() } @@ -1450,6 +1582,61 @@ verification_token = "notion-verification-token-example" assert!(cfg.notion.is_none()); } + #[test] + fn sentry_resolves_with_valid_secret() { + let f = write_toml(&sentry_toml("sentry-client-secret")); + let cfg = load(Some(f.path())).expect("load failed"); + assert!(cfg.sentry.is_some()); + } + + #[test] + fn sentry_disabled_returns_none() { + let toml = r#" +[sources.sentry] +status = "disabled" +client_secret = "sentry-client-secret" +"#; + let f = write_toml(toml); + let cfg = load(Some(f.path())).expect("load failed"); + assert!(cfg.sentry.is_none()); + } + + #[test] + fn sentry_missing_client_secret_returns_none_when_status_unspecified() { + let toml = r#" +[sources.sentry] +"#; + let f = write_toml(toml); + let cfg = load(Some(f.path())).expect("load failed"); + assert!(cfg.sentry.is_none()); + } + + #[test] + fn sentry_enabled_without_client_secret_is_invalid() { + let toml = r#" +[sources.sentry] +status = "enabled" +"#; + let f = write_toml(toml); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: missing client_secret"))) + ); + } + + #[test] + fn sentry_enabled_with_surrounding_whitespace_without_client_secret_is_invalid() { + let toml = r#" +[sources.sentry] +status = " enabled " +"#; + let f = write_toml(toml); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: missing client_secret"))) + ); + } + #[test] fn notion_missing_token_returns_none() { let toml = r#" @@ -1901,6 +2088,14 @@ webhook_secret = "gh-secret" assert!(err.source().is_none()); } + #[test] + fn config_validation_error_missing_field_has_no_source() { + let err = ConfigValidationError::missing("sentry", "client_secret"); + + assert_eq!(err.to_string(), "sentry: missing client_secret"); + assert!(err.source().is_none()); + } + #[test] fn config_error_is_std_error() { let err = ConfigError::Validation(vec![ConfigValidationError::invalid( @@ -1992,6 +2187,15 @@ port = 9090 ); } + #[test] + fn sentry_empty_client_secret_is_invalid() { + let f = write_toml(&sentry_toml("")); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: invalid client_secret"))) + ); + } + #[test] fn incidentio_invalid_secret_is_invalid() { let f = write_toml(&incidentio_toml("whsec_not-base64!")); @@ -2125,6 +2329,20 @@ subject_prefix = "has.dots" ); } + #[test] + fn sentry_invalid_subject_prefix() { + let toml = r#" +[sources.sentry] +client_secret = "sentry-client-secret" +subject_prefix = "has.dots" +"#; + let f = write_toml(toml); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: invalid subject_prefix"))) + ); + } + #[test] fn slack_invalid_stream_name() { let toml = r#" @@ -2212,6 +2430,20 @@ stream_name = "has.dots" ); } + #[test] + fn sentry_invalid_stream_name() { + let toml = r#" +[sources.sentry] +client_secret = "sentry-client-secret" +stream_name = "has.dots" +"#; + let f = write_toml(toml); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: invalid stream_name"))) + ); + } + #[test] fn incidentio_zero_nats_ack_timeout_is_error() { let toml = format!( @@ -2274,6 +2506,34 @@ stream_max_age_secs = 0 ); } + #[test] + fn sentry_zero_nats_ack_timeout_is_error() { + let toml = r#" +[sources.sentry] +client_secret = "sentry-client-secret" +nats_ack_timeout_secs = 0 +"#; + let f = write_toml(toml); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: nats_ack_timeout_secs must not be zero"))) + ); + } + + #[test] + fn sentry_zero_stream_max_age_is_error() { + let toml = r#" +[sources.sentry] +client_secret = "sentry-client-secret" +stream_max_age_secs = 0 +"#; + let f = write_toml(toml); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: stream_max_age_secs must not be zero"))) + ); + } + #[test] fn incidentio_zero_timestamp_tolerance_is_error() { let toml = format!( diff --git a/rsworkspace/crates/trogon-gateway/src/http.rs b/rsworkspace/crates/trogon-gateway/src/http.rs index 37be4be4d..9c7255d6e 100644 --- a/rsworkspace/crates/trogon-gateway/src/http.rs +++ b/rsworkspace/crates/trogon-gateway/src/http.rs @@ -78,6 +78,14 @@ where info!(source = "notion", "mounted at /notion"); } + if let Some(ref cfg) = config.sentry { + app = app.nest( + "/sentry", + trogon_source_sentry::router(publisher.clone(), cfg), + ); + info!(source = "sentry", "mounted at /sentry"); + } + app } @@ -137,6 +145,9 @@ webhook_secret = "linear-secret" [sources.notion] verification_token = "notion-verification-token-example" + +[sources.sentry] +client_secret = "sentry-client-secret" "# .to_string() } diff --git a/rsworkspace/crates/trogon-gateway/src/streams.rs b/rsworkspace/crates/trogon-gateway/src/streams.rs index 59e31a4e8..f5476cb3e 100644 --- a/rsworkspace/crates/trogon-gateway/src/streams.rs +++ b/rsworkspace/crates/trogon-gateway/src/streams.rs @@ -39,6 +39,10 @@ pub(crate) async fn provision( trogon_source_notion::provision(client, cfg).await?; info!(source = "notion", "stream provisioned"); } + if let Some(ref cfg) = config.sentry { + trogon_source_sentry::provision(client, cfg).await?; + info!(source = "sentry", "stream provisioned"); + } Ok(()) } @@ -85,6 +89,9 @@ webhook_secret = "linear-secret" [sources.notion] verification_token = "notion-verification-token-example" + +[sources.sentry] +client_secret = "sentry-client-secret" "# .to_string() } @@ -111,7 +118,7 @@ verification_token = "notion-verification-token-example" .await .expect("provision should succeed"); - assert_eq!(js.created_streams().len(), 8); + assert_eq!(js.created_streams().len(), 9); } #[tokio::test] @@ -141,6 +148,9 @@ webhook_secret = "linear-secret" [sources.notion] verification_token = "notion-verification-token-example" + +[sources.sentry] +client_secret = "sentry-client-secret" "#; let f = write_toml(toml); let cfg = load(Some(f.path())).expect("load failed"); @@ -150,6 +160,6 @@ verification_token = "notion-verification-token-example" .await .expect("provision should succeed"); - assert_eq!(js.created_streams().len(), 7); + assert_eq!(js.created_streams().len(), 8); } } diff --git a/rsworkspace/crates/trogon-source-sentry/Cargo.toml b/rsworkspace/crates/trogon-source-sentry/Cargo.toml new file mode 100644 index 000000000..0278a0210 --- /dev/null +++ b/rsworkspace/crates/trogon-source-sentry/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "trogon-source-sentry" +version = "0.1.0" +edition = "2024" + +[lints] +workspace = true + +[dependencies] +async-nats = { workspace = true, features = ["jetstream"] } +axum = { workspace = true } +hex = "0.4" +hmac = "0.12" +serde = { workspace = true } +serde_json = { workspace = true } +sha2 = "0.10" +tracing = { workspace = true } +trogon-nats = { workspace = true } +trogon-std = { workspace = true } + +[dev-dependencies] +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +tower = "0.5" +tracing-subscriber = { workspace = true } +trogon-nats = { workspace = true, features = ["test-support"] } diff --git a/rsworkspace/crates/trogon-source-sentry/src/config.rs b/rsworkspace/crates/trogon-source-sentry/src/config.rs new file mode 100644 index 000000000..407071cd3 --- /dev/null +++ b/rsworkspace/crates/trogon-source-sentry/src/config.rs @@ -0,0 +1,12 @@ +use crate::sentry_client_secret::SentryClientSecret; +use trogon_nats::NatsToken; +use trogon_nats::jetstream::StreamMaxAge; +use trogon_std::NonZeroDuration; + +pub struct SentryConfig { + pub client_secret: SentryClientSecret, + pub subject_prefix: NatsToken, + pub stream_name: NatsToken, + pub stream_max_age: StreamMaxAge, + pub nats_ack_timeout: NonZeroDuration, +} diff --git a/rsworkspace/crates/trogon-source-sentry/src/constants.rs b/rsworkspace/crates/trogon-source-sentry/src/constants.rs new file mode 100644 index 000000000..518dc42d6 --- /dev/null +++ b/rsworkspace/crates/trogon-source-sentry/src/constants.rs @@ -0,0 +1,13 @@ +use trogon_std::{ByteSize, HttpBodySizeMax}; + +pub const HTTP_BODY_SIZE_MAX: HttpBodySizeMax = HttpBodySizeMax::new(ByteSize::mib(2)).unwrap(); + +pub const HEADER_RESOURCE: &str = "sentry-hook-resource"; +pub const HEADER_SIGNATURE: &str = "sentry-hook-signature"; +pub const HEADER_TIMESTAMP: &str = "sentry-hook-timestamp"; +pub const HEADER_REQUEST_ID: &str = "request-id"; + +pub const NATS_HEADER_ACTION: &str = "X-Sentry-Action"; +pub const NATS_HEADER_REQUEST_ID: &str = "X-Sentry-Request-Id"; +pub const NATS_HEADER_RESOURCE: &str = "X-Sentry-Hook-Resource"; +pub const NATS_HEADER_TIMESTAMP: &str = "X-Sentry-Hook-Timestamp"; diff --git a/rsworkspace/crates/trogon-source-sentry/src/lib.rs b/rsworkspace/crates/trogon-source-sentry/src/lib.rs new file mode 100644 index 000000000..00316a00a --- /dev/null +++ b/rsworkspace/crates/trogon-source-sentry/src/lib.rs @@ -0,0 +1,10 @@ +pub mod config; +pub mod constants; +pub mod sentry_client_secret; +pub mod server; +pub mod signature; + +pub use config::SentryConfig; +pub use sentry_client_secret::SentryClientSecret; +pub use server::{provision, router}; +pub use signature::SignatureError; diff --git a/rsworkspace/crates/trogon-source-sentry/src/sentry_client_secret.rs b/rsworkspace/crates/trogon-source-sentry/src/sentry_client_secret.rs new file mode 100644 index 000000000..c98769e8d --- /dev/null +++ b/rsworkspace/crates/trogon-source-sentry/src/sentry_client_secret.rs @@ -0,0 +1,39 @@ +use std::fmt; + +use trogon_std::{EmptySecret, SecretString}; + +#[derive(Clone)] +pub struct SentryClientSecret(SecretString); + +impl SentryClientSecret { + pub fn new(s: impl AsRef) -> Result { + SecretString::new(s).map(Self) + } + + pub fn as_str(&self) -> &str { + self.0.as_str() + } +} + +impl fmt::Debug for SentryClientSecret { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("SentryClientSecret(****)") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sentry_client_secret_roundtrips() { + let secret = SentryClientSecret::new("super-secret").unwrap(); + assert_eq!(secret.as_str(), "super-secret"); + } + + #[test] + fn sentry_client_secret_debug_redacts() { + let secret = SentryClientSecret::new("super-secret").unwrap(); + assert_eq!(format!("{secret:?}"), "SentryClientSecret(****)"); + } +} diff --git a/rsworkspace/crates/trogon-source-sentry/src/server.rs b/rsworkspace/crates/trogon-source-sentry/src/server.rs new file mode 100644 index 000000000..a43f015f3 --- /dev/null +++ b/rsworkspace/crates/trogon-source-sentry/src/server.rs @@ -0,0 +1,476 @@ +use std::fmt; +use std::time::Duration; + +use crate::config::SentryConfig; +use crate::constants::{ + HEADER_REQUEST_ID, HEADER_RESOURCE, HEADER_SIGNATURE, HEADER_TIMESTAMP, HTTP_BODY_SIZE_MAX, + NATS_HEADER_ACTION, NATS_HEADER_REQUEST_ID, NATS_HEADER_RESOURCE, NATS_HEADER_TIMESTAMP, +}; +use crate::signature; +use axum::{ + Router, body::Bytes, extract::DefaultBodyLimit, extract::State, http::HeaderMap, + http::StatusCode, routing::post, +}; +use serde::Deserialize; +use tracing::{info, instrument, warn}; +use trogon_nats::NatsToken; +use trogon_nats::jetstream::{ + ClaimCheckPublisher, JetStreamContext, JetStreamPublisher, ObjectStorePut, PublishOutcome, +}; +use trogon_std::NonZeroDuration; + +#[derive(Deserialize)] +struct WebhookEnvelope { + action: String, +} + +fn outcome_to_status(outcome: PublishOutcome) -> StatusCode { + if outcome.is_ok() { + info!("Published Sentry event to NATS"); + StatusCode::OK + } else { + outcome.log_on_error("sentry"); + StatusCode::INTERNAL_SERVER_ERROR + } +} + +#[derive(Clone)] +struct AppState { + publisher: ClaimCheckPublisher, + client_secret: crate::sentry_client_secret::SentryClientSecret, + subject_prefix: NatsToken, + nats_ack_timeout: NonZeroDuration, +} + +pub async fn provision(js: &C, config: &SentryConfig) -> Result<(), C::Error> { + js.get_or_create_stream(async_nats::jetstream::stream::Config { + name: config.stream_name.as_str().to_owned(), + subjects: vec![format!("{}.>", config.subject_prefix)], + max_age: config.stream_max_age.into(), + ..Default::default() + }) + .await?; + + let stream = config.stream_name.as_str(); + let max_age_secs = Duration::from(config.stream_max_age).as_secs(); + info!(stream, max_age_secs, "JetStream stream ready"); + Ok(()) +} + +pub fn router( + publisher: ClaimCheckPublisher, + config: &SentryConfig, +) -> Router { + let state = AppState { + publisher, + client_secret: config.client_secret.clone(), + subject_prefix: config.subject_prefix.clone(), + nats_ack_timeout: config.nats_ack_timeout, + }; + + Router::new() + .route("/webhook", post(handle_webhook::)) + .layer(DefaultBodyLimit::max(HTTP_BODY_SIZE_MAX.as_usize())) + .with_state(state) +} + +#[instrument( + name = "sentry.webhook", + skip_all, + fields( + resource = tracing::field::Empty, + action = tracing::field::Empty, + request_id = tracing::field::Empty, + subject = tracing::field::Empty, + ) +)] +async fn handle_webhook( + State(state): State>, + headers: HeaderMap, + body: Bytes, +) -> StatusCode { + let Some(signature) = headers + .get(HEADER_SIGNATURE) + .and_then(|value| value.to_str().ok()) + else { + warn!("Missing Sentry-Hook-Signature header"); + return StatusCode::UNAUTHORIZED; + }; + + if let Err(error) = signature::verify(state.client_secret.as_str(), &body, signature) { + warn!(reason = %error, "Sentry webhook signature validation failed"); + return StatusCode::UNAUTHORIZED; + } + + let Some(resource) = headers + .get(HEADER_RESOURCE) + .and_then(|value| value.to_str().ok()) + .map(str::to_owned) + else { + warn!("Missing Sentry-Hook-Resource header"); + return StatusCode::BAD_REQUEST; + }; + + let timestamp = headers + .get(HEADER_TIMESTAMP) + .and_then(|value| value.to_str().ok()) + .map(str::to_owned); + let request_id = headers + .get(HEADER_REQUEST_ID) + .and_then(|value| value.to_str().ok()) + .map(str::to_owned); + + let payload = match serde_json::from_slice::(&body) { + Ok(payload) => payload, + Err(error) => { + warn!(error = %error, "Failed to parse Sentry webhook payload"); + return StatusCode::BAD_REQUEST; + } + }; + + let resource_token = match NatsToken::new(resource.as_str()) { + Ok(token) => token, + Err(error) => { + warn!(reason = ?error, resource = %resource, "Invalid Sentry resource token"); + return StatusCode::BAD_REQUEST; + } + }; + let action_token = match NatsToken::new(payload.action.as_str()) { + Ok(token) => token, + Err(error) => { + warn!(reason = ?error, action = %payload.action, "Invalid Sentry action token"); + return StatusCode::BAD_REQUEST; + } + }; + + let subject = format!( + "{}.{}.{}", + state.subject_prefix, resource_token, action_token + ); + let span = tracing::Span::current(); + span.record("resource", &resource); + span.record("action", &payload.action); + span.record("request_id", request_id.as_deref().unwrap_or("unknown")); + span.record("subject", &subject); + + let mut nats_headers = async_nats::HeaderMap::new(); + nats_headers.insert(NATS_HEADER_RESOURCE, resource.as_str()); + nats_headers.insert(NATS_HEADER_ACTION, payload.action.as_str()); + if let Some(ref request_id) = request_id { + nats_headers.insert(async_nats::header::NATS_MESSAGE_ID, request_id.as_str()); + nats_headers.insert(NATS_HEADER_REQUEST_ID, request_id.as_str()); + } + if let Some(ref timestamp) = timestamp { + nats_headers.insert(NATS_HEADER_TIMESTAMP, timestamp.as_str()); + } + + let outcome = state + .publisher + .publish_event(subject, nats_headers, body, state.nats_ack_timeout.into()) + .await; + + outcome_to_status(outcome) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::SentryConfig; + use crate::constants::{ + HEADER_REQUEST_ID, HEADER_RESOURCE, HEADER_SIGNATURE, HEADER_TIMESTAMP, NATS_HEADER_ACTION, + NATS_HEADER_REQUEST_ID, NATS_HEADER_RESOURCE, NATS_HEADER_TIMESTAMP, + }; + use crate::sentry_client_secret::SentryClientSecret; + use axum::body::Body; + use axum::http::Request; + use hmac::{Hmac, Mac}; + use sha2::Sha256; + use tower::ServiceExt; + use tracing_subscriber::util::SubscriberInitExt; + use trogon_nats::jetstream::StreamMaxAge; + use trogon_nats::jetstream::{ + ClaimCheckPublisher, MaxPayload, MockJetStreamContext, MockJetStreamPublisher, + MockObjectStore, + }; + + type HmacSha256 = Hmac; + + const TEST_SECRET: &str = "test-secret"; + + fn wrap_publisher( + publisher: MockJetStreamPublisher, + ) -> ClaimCheckPublisher { + ClaimCheckPublisher::new( + publisher, + MockObjectStore::new(), + "test-bucket".to_string(), + MaxPayload::from_server_limit(usize::MAX), + ) + } + + fn compute_sig(secret: &str, body: &[u8]) -> String { + let mut mac = HmacSha256::new_from_slice(secret.as_bytes()).unwrap(); + mac.update(body); + hex::encode(mac.finalize().into_bytes()) + } + + fn test_config() -> SentryConfig { + SentryConfig { + client_secret: SentryClientSecret::new(TEST_SECRET).unwrap(), + subject_prefix: NatsToken::new("sentry").unwrap(), + stream_name: NatsToken::new("SENTRY").unwrap(), + stream_max_age: StreamMaxAge::from_secs(3600).unwrap(), + nats_ack_timeout: NonZeroDuration::from_secs(10).unwrap(), + } + } + + fn tracing_guard() -> tracing::subscriber::DefaultGuard { + tracing_subscriber::fmt().with_test_writer().set_default() + } + + fn mock_app(publisher: MockJetStreamPublisher) -> Router { + router(wrap_publisher(publisher), &test_config()) + } + + fn webhook_request( + body: &[u8], + resource: &str, + timestamp: &str, + request_id: &str, + signature: Option<&str>, + ) -> Request { + let mut builder = Request::builder() + .method("POST") + .uri("/webhook") + .header(HEADER_RESOURCE, resource) + .header(HEADER_TIMESTAMP, timestamp) + .header(HEADER_REQUEST_ID, request_id); + + if let Some(signature) = signature { + builder = builder.header(HEADER_SIGNATURE, signature); + } + + builder.body(Body::from(body.to_vec())).unwrap() + } + + #[tokio::test] + async fn provision_creates_stream() { + let _guard = tracing_guard(); + let js = MockJetStreamContext::new(); + let config = test_config(); + + provision(&js, &config).await.unwrap(); + + let streams = js.created_streams(); + assert_eq!(streams.len(), 1); + assert_eq!(streams[0].name, "SENTRY"); + assert_eq!(streams[0].subjects, vec!["sentry.>"]); + assert_eq!(streams[0].max_age, Duration::from_secs(3600)); + } + + #[tokio::test] + async fn provision_propagates_error() { + let _guard = tracing_guard(); + let js = MockJetStreamContext::new(); + js.fail_next(); + let config = test_config(); + + let result = provision(&js, &config).await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn valid_webhook_publishes_to_nats_and_returns_200() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"created","data":{}}"#; + let signature = compute_sig(TEST_SECRET, body); + + let response = app + .oneshot(webhook_request( + body, + "issue", + "1711315768", + "req-1", + Some(&signature), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let messages = publisher.published_messages(); + assert_eq!(messages.len(), 1); + assert_eq!(messages[0].subject, "sentry.issue.created"); + assert_eq!(messages[0].payload.as_ref(), body); + assert_eq!( + messages[0] + .headers + .get(async_nats::header::NATS_MESSAGE_ID) + .unwrap() + .as_str(), + "req-1" + ); + assert_eq!( + messages[0] + .headers + .get(NATS_HEADER_RESOURCE) + .unwrap() + .as_str(), + "issue" + ); + assert_eq!( + messages[0] + .headers + .get(NATS_HEADER_ACTION) + .unwrap() + .as_str(), + "created" + ); + assert_eq!( + messages[0] + .headers + .get(NATS_HEADER_REQUEST_ID) + .unwrap() + .as_str(), + "req-1" + ); + assert_eq!( + messages[0] + .headers + .get(NATS_HEADER_TIMESTAMP) + .unwrap() + .as_str(), + "1711315768" + ); + } + + #[tokio::test] + async fn missing_signature_returns_401() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"created"}"#; + + let response = app + .oneshot(webhook_request(body, "issue", "1711315768", "req-1", None)) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!(publisher.published_messages().is_empty()); + } + + #[tokio::test] + async fn invalid_signature_returns_401() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"created"}"#; + + let response = app + .oneshot(webhook_request( + body, + "issue", + "1711315768", + "req-1", + Some("not-valid"), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!(publisher.published_messages().is_empty()); + } + + #[tokio::test] + async fn missing_resource_returns_400() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"created"}"#; + let signature = compute_sig(TEST_SECRET, body); + + let request = Request::builder() + .method("POST") + .uri("/webhook") + .header(HEADER_SIGNATURE, signature) + .body(Body::from(body.to_vec())) + .unwrap(); + + let response = app.oneshot(request).await.unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert!(publisher.published_messages().is_empty()); + } + + #[tokio::test] + async fn invalid_payload_returns_400() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"not-json"#; + let signature = compute_sig(TEST_SECRET, body); + + let response = app + .oneshot(webhook_request( + body, + "issue", + "1711315768", + "req-1", + Some(&signature), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert!(publisher.published_messages().is_empty()); + } + + #[tokio::test] + async fn invalid_action_token_returns_400() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"issue.created"}"#; + let signature = compute_sig(TEST_SECRET, body); + + let response = app + .oneshot(webhook_request( + body, + "issue", + "1711315768", + "req-1", + Some(&signature), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert!(publisher.published_messages().is_empty()); + } + + #[tokio::test] + async fn publish_error_returns_500() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + publisher.fail_next_js_publish(); + let app = mock_app(publisher); + let body = br#"{"action":"created"}"#; + let signature = compute_sig(TEST_SECRET, body); + + let response = app + .oneshot(webhook_request( + body, + "issue", + "1711315768", + "req-1", + Some(&signature), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + } +} diff --git a/rsworkspace/crates/trogon-source-sentry/src/signature.rs b/rsworkspace/crates/trogon-source-sentry/src/signature.rs new file mode 100644 index 000000000..f8f368bb7 --- /dev/null +++ b/rsworkspace/crates/trogon-source-sentry/src/signature.rs @@ -0,0 +1,104 @@ +use std::fmt; + +use hmac::{Hmac, Mac}; +use sha2::Sha256; + +type HmacSha256 = Hmac; + +#[derive(Debug)] +#[non_exhaustive] +pub enum SignatureError { + InvalidHex(hex::FromHexError), + InvalidKey, + Mismatch, +} + +impl fmt::Display for SignatureError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + SignatureError::InvalidHex(_) => f.write_str("invalid hex encoding"), + SignatureError::InvalidKey => f.write_str("invalid HMAC key"), + SignatureError::Mismatch => f.write_str("signature mismatch"), + } + } +} + +impl std::error::Error for SignatureError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + SignatureError::InvalidHex(error) => Some(error), + SignatureError::InvalidKey => None, + SignatureError::Mismatch => None, + } + } +} + +pub fn verify(secret: &str, body: &[u8], signature_header: &str) -> Result<(), SignatureError> { + let expected = hex::decode(signature_header).map_err(SignatureError::InvalidHex)?; + let mut mac = + HmacSha256::new_from_slice(secret.as_bytes()).map_err(|_| SignatureError::InvalidKey)?; + mac.update(body); + mac.verify_slice(&expected) + .map_err(|_| SignatureError::Mismatch) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::error::Error; + + fn compute_sig(secret: &str, body: &[u8]) -> String { + let mut mac = HmacSha256::new_from_slice(secret.as_bytes()).unwrap(); + mac.update(body); + hex::encode(mac.finalize().into_bytes()) + } + + #[test] + fn error_display_messages() { + let hex_error = SignatureError::InvalidHex(hex::decode("zz").unwrap_err()); + assert_eq!(hex_error.to_string(), "invalid hex encoding"); + assert!(hex_error.source().is_some()); + assert_eq!(SignatureError::InvalidKey.to_string(), "invalid HMAC key"); + assert!(SignatureError::InvalidKey.source().is_none()); + assert_eq!(SignatureError::Mismatch.to_string(), "signature mismatch"); + assert!(SignatureError::Mismatch.source().is_none()); + } + + #[test] + fn valid_signature_passes() { + let sig = compute_sig("test-secret", b"hello world"); + assert!(verify("test-secret", b"hello world", &sig).is_ok()); + } + + #[test] + fn wrong_secret_fails() { + let sig = compute_sig("correct-secret", b"body"); + assert!(matches!( + verify("wrong-secret", b"body", &sig), + Err(SignatureError::Mismatch) + )); + } + + #[test] + fn tampered_body_fails() { + let sig = compute_sig("secret", b"original body"); + assert!(matches!( + verify("secret", b"tampered body", &sig), + Err(SignatureError::Mismatch) + )); + } + + #[test] + fn invalid_hex_fails() { + assert!(matches!( + verify("secret", b"body", "not-valid-hex!"), + Err(SignatureError::InvalidHex(_)) + )); + } + + #[test] + fn empty_body_with_valid_sig_passes() { + let sig = compute_sig("secret", b""); + assert!(verify("secret", b"", &sig).is_ok()); + } +} From d134018e7df8c120f5890b7e4566e89d925cebf0 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Wed, 15 Apr 2026 01:36:17 -0400 Subject: [PATCH 2/3] fix(sentry): honor the documented webhook contract Signed-off-by: Yordis Prieto --- devops/docker/compose/.env.example | 2 +- .../crates/trogon-gateway/src/config.rs | 49 ++++- .../crates/trogon-source-sentry/src/server.rs | 182 +++++++++++++++--- 3 files changed, 201 insertions(+), 32 deletions(-) diff --git a/devops/docker/compose/.env.example b/devops/docker/compose/.env.example index 38f9d9565..7ee6ac0b0 100644 --- a/devops/docker/compose/.env.example +++ b/devops/docker/compose/.env.example @@ -46,7 +46,7 @@ # TROGON_SOURCE_SENTRY_SUBJECT_PREFIX=sentry # TROGON_SOURCE_SENTRY_STREAM_NAME=SENTRY # TROGON_SOURCE_SENTRY_STREAM_MAX_AGE_SECS=604800 -# TROGON_SOURCE_SENTRY_NATS_ACK_TIMEOUT_SECS=10 +# TROGON_SOURCE_SENTRY_NATS_ACK_TIMEOUT_SECS=1 # --- Discord Source --- # TROGON_SOURCE_DISCORD_BOT_TOKEN= diff --git a/rsworkspace/crates/trogon-gateway/src/config.rs b/rsworkspace/crates/trogon-gateway/src/config.rs index 2ff39c01a..6451fb382 100644 --- a/rsworkspace/crates/trogon-gateway/src/config.rs +++ b/rsworkspace/crates/trogon-gateway/src/config.rs @@ -33,6 +33,31 @@ impl fmt::Display for ZeroNotAllowed { impl std::error::Error for ZeroNotAllowed {} +#[derive(Debug)] +struct DurationTooLong { + max_secs: u64, +} + +impl DurationTooLong { + fn new(max_secs: u64) -> Self { + Self { max_secs } + } +} + +impl fmt::Display for DurationTooLong { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.max_secs == 1 { + f.write_str("must not exceed 1 second") + } else { + write!(f, "must not exceed {} seconds", self.max_secs) + } + } +} + +impl std::error::Error for DurationTooLong {} + +const SENTRY_MAX_ACK_TIMEOUT_SECS: u64 = 1; + #[derive(Debug)] pub enum ConfigValidationError { InvalidField { @@ -341,7 +366,7 @@ struct SentryConfig { stream_name: String, #[config(env = "TROGON_SOURCE_SENTRY_STREAM_MAX_AGE_SECS", default = 604_800)] stream_max_age_secs: u64, - #[config(env = "TROGON_SOURCE_SENTRY_NATS_ACK_TIMEOUT_SECS", default = 10)] + #[config(env = "TROGON_SOURCE_SENTRY_NATS_ACK_TIMEOUT_SECS", default = 1)] nats_ack_timeout_secs: u64, } @@ -1173,6 +1198,14 @@ fn resolve_sentry( return None; } }; + if section.nats_ack_timeout_secs > SENTRY_MAX_ACK_TIMEOUT_SECS { + errors.push(ConfigValidationError::invalid( + "sentry", + "nats_ack_timeout_secs", + DurationTooLong::new(SENTRY_MAX_ACK_TIMEOUT_SECS), + )); + return None; + } let stream_max_age = match StreamMaxAge::from_secs(section.stream_max_age_secs) { Ok(age) => age, @@ -2534,6 +2567,20 @@ stream_max_age_secs = 0 ); } + #[test] + fn sentry_nats_ack_timeout_over_one_second_is_error() { + let toml = r#" +[sources.sentry] +client_secret = "sentry-client-secret" +nats_ack_timeout_secs = 2 +"#; + let f = write_toml(toml); + let result = load(Some(f.path())); + assert!( + matches!(result, Err(ConfigError::Validation(ref errs)) if errs.iter().any(|e| e.contains("sentry: invalid nats_ack_timeout_secs: must not exceed 1 second"))) + ); + } + #[test] fn incidentio_zero_timestamp_tolerance_is_error() { let toml = format!( diff --git a/rsworkspace/crates/trogon-source-sentry/src/server.rs b/rsworkspace/crates/trogon-source-sentry/src/server.rs index a43f015f3..33583f43c 100644 --- a/rsworkspace/crates/trogon-source-sentry/src/server.rs +++ b/rsworkspace/crates/trogon-source-sentry/src/server.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::str::FromStr; use std::time::Duration; use crate::config::SentryConfig; @@ -24,6 +25,67 @@ struct WebhookEnvelope { action: String, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum SentryResource { + Installation, + EventAlert, + Issue, + MetricAlert, + Error, + Comment, + Seer, +} + +impl SentryResource { + fn as_str(self) -> &'static str { + match self { + Self::Installation => "installation", + Self::EventAlert => "event_alert", + Self::Issue => "issue", + Self::MetricAlert => "metric_alert", + Self::Error => "error", + Self::Comment => "comment", + Self::Seer => "seer", + } + } +} + +#[derive(Debug)] +struct InvalidSentryResource; + +impl fmt::Display for InvalidSentryResource { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("unsupported Sentry webhook resource") + } +} + +impl std::error::Error for InvalidSentryResource {} + +impl FromStr for SentryResource { + type Err = InvalidSentryResource; + + fn from_str(value: &str) -> Result { + match value.trim() { + "installation" => Ok(Self::Installation), + "event_alert" => Ok(Self::EventAlert), + "issue" => Ok(Self::Issue), + "metric_alert" => Ok(Self::MetricAlert), + "error" => Ok(Self::Error), + "comment" => Ok(Self::Comment), + "seer" => Ok(Self::Seer), + _ => Err(InvalidSentryResource), + } + } +} + +fn header_value<'a>(headers: &'a HeaderMap, name: &'static str) -> Option<&'a str> { + headers + .get(name) + .and_then(|value| value.to_str().ok()) + .map(str::trim) + .filter(|value| !value.is_empty()) +} + fn outcome_to_status(outcome: PublishOutcome) -> StatusCode { if outcome.is_ok() { info!("Published Sentry event to NATS"); @@ -89,10 +151,7 @@ async fn handle_webhook( headers: HeaderMap, body: Bytes, ) -> StatusCode { - let Some(signature) = headers - .get(HEADER_SIGNATURE) - .and_then(|value| value.to_str().ok()) - else { + let Some(signature) = header_value(&headers, HEADER_SIGNATURE) else { warn!("Missing Sentry-Hook-Signature header"); return StatusCode::UNAUTHORIZED; }; @@ -102,23 +161,19 @@ async fn handle_webhook( return StatusCode::UNAUTHORIZED; } - let Some(resource) = headers - .get(HEADER_RESOURCE) - .and_then(|value| value.to_str().ok()) - .map(str::to_owned) - else { + let Some(resource) = header_value(&headers, HEADER_RESOURCE) else { warn!("Missing Sentry-Hook-Resource header"); return StatusCode::BAD_REQUEST; }; - let timestamp = headers - .get(HEADER_TIMESTAMP) - .and_then(|value| value.to_str().ok()) - .map(str::to_owned); - let request_id = headers - .get(HEADER_REQUEST_ID) - .and_then(|value| value.to_str().ok()) - .map(str::to_owned); + let Some(timestamp) = header_value(&headers, HEADER_TIMESTAMP).map(str::to_owned) else { + warn!("Missing Sentry-Hook-Timestamp header"); + return StatusCode::BAD_REQUEST; + }; + let Some(request_id) = header_value(&headers, HEADER_REQUEST_ID).map(str::to_owned) else { + warn!("Missing Request-ID header"); + return StatusCode::BAD_REQUEST; + }; let payload = match serde_json::from_slice::(&body) { Ok(payload) => payload, @@ -128,10 +183,10 @@ async fn handle_webhook( } }; - let resource_token = match NatsToken::new(resource.as_str()) { - Ok(token) => token, + let resource = match resource.parse::() { + Ok(resource) => resource, Err(error) => { - warn!(reason = ?error, resource = %resource, "Invalid Sentry resource token"); + warn!(reason = %error, resource, "Unsupported Sentry webhook resource"); return StatusCode::BAD_REQUEST; } }; @@ -145,24 +200,22 @@ async fn handle_webhook( let subject = format!( "{}.{}.{}", - state.subject_prefix, resource_token, action_token + state.subject_prefix, + resource.as_str(), + action_token ); let span = tracing::Span::current(); - span.record("resource", &resource); + span.record("resource", resource.as_str()); span.record("action", &payload.action); - span.record("request_id", request_id.as_deref().unwrap_or("unknown")); + span.record("request_id", &request_id); span.record("subject", &subject); let mut nats_headers = async_nats::HeaderMap::new(); nats_headers.insert(NATS_HEADER_RESOURCE, resource.as_str()); nats_headers.insert(NATS_HEADER_ACTION, payload.action.as_str()); - if let Some(ref request_id) = request_id { - nats_headers.insert(async_nats::header::NATS_MESSAGE_ID, request_id.as_str()); - nats_headers.insert(NATS_HEADER_REQUEST_ID, request_id.as_str()); - } - if let Some(ref timestamp) = timestamp { - nats_headers.insert(NATS_HEADER_TIMESTAMP, timestamp.as_str()); - } + nats_headers.insert(async_nats::header::NATS_MESSAGE_ID, request_id.as_str()); + nats_headers.insert(NATS_HEADER_REQUEST_ID, request_id.as_str()); + nats_headers.insert(NATS_HEADER_TIMESTAMP, timestamp.as_str()); let outcome = state .publisher @@ -405,6 +458,52 @@ mod tests { assert!(publisher.published_messages().is_empty()); } + #[tokio::test] + async fn missing_timestamp_returns_400() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"created"}"#; + let signature = compute_sig(TEST_SECRET, body); + + let request = Request::builder() + .method("POST") + .uri("/webhook") + .header(HEADER_RESOURCE, "issue") + .header(HEADER_REQUEST_ID, "req-1") + .header(HEADER_SIGNATURE, signature) + .body(Body::from(body.to_vec())) + .unwrap(); + + let response = app.oneshot(request).await.unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert!(publisher.published_messages().is_empty()); + } + + #[tokio::test] + async fn missing_request_id_returns_400() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"created"}"#; + let signature = compute_sig(TEST_SECRET, body); + + let request = Request::builder() + .method("POST") + .uri("/webhook") + .header(HEADER_RESOURCE, "issue") + .header(HEADER_TIMESTAMP, "1711315768") + .header(HEADER_SIGNATURE, signature) + .body(Body::from(body.to_vec())) + .unwrap(); + + let response = app.oneshot(request).await.unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert!(publisher.published_messages().is_empty()); + } + #[tokio::test] async fn invalid_payload_returns_400() { let _guard = tracing_guard(); @@ -428,6 +527,29 @@ mod tests { assert!(publisher.published_messages().is_empty()); } + #[tokio::test] + async fn unsupported_resource_returns_400() { + let _guard = tracing_guard(); + let publisher = MockJetStreamPublisher::new(); + let app = mock_app(publisher.clone()); + let body = br#"{"action":"created"}"#; + let signature = compute_sig(TEST_SECRET, body); + + let response = app + .oneshot(webhook_request( + body, + "organization", + "1711315768", + "req-1", + Some(&signature), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert!(publisher.published_messages().is_empty()); + } + #[tokio::test] async fn invalid_action_token_returns_400() { let _guard = tracing_guard(); From 151b6dd71981aae0b01c20d893aea3be45a762b0 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Wed, 15 Apr 2026 11:04:09 -0400 Subject: [PATCH 3/3] fix(gateway): keep new Sentry validation from tripping the coverage gate Signed-off-by: Yordis Prieto --- rsworkspace/crates/trogon-gateway/src/config.rs | 7 +++++++ rsworkspace/crates/trogon-source-sentry/src/server.rs | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/rsworkspace/crates/trogon-gateway/src/config.rs b/rsworkspace/crates/trogon-gateway/src/config.rs index 6451fb382..115500115 100644 --- a/rsworkspace/crates/trogon-gateway/src/config.rs +++ b/rsworkspace/crates/trogon-gateway/src/config.rs @@ -2129,6 +2129,13 @@ webhook_secret = "gh-secret" assert!(err.source().is_none()); } + #[test] + fn duration_too_long_display_uses_plural_for_values_above_one_second() { + let err = DurationTooLong::new(2); + + assert_eq!(err.to_string(), "must not exceed 2 seconds"); + } + #[test] fn config_error_is_std_error() { let err = ConfigError::Validation(vec![ConfigValidationError::invalid( diff --git a/rsworkspace/crates/trogon-source-sentry/src/server.rs b/rsworkspace/crates/trogon-source-sentry/src/server.rs index 33583f43c..e9ff480d3 100644 --- a/rsworkspace/crates/trogon-source-sentry/src/server.rs +++ b/rsworkspace/crates/trogon-source-sentry/src/server.rs @@ -550,6 +550,17 @@ mod tests { assert!(publisher.published_messages().is_empty()); } + #[test] + fn sentry_resource_as_str_covers_all_documented_values() { + assert_eq!(SentryResource::Installation.as_str(), "installation"); + assert_eq!(SentryResource::EventAlert.as_str(), "event_alert"); + assert_eq!(SentryResource::Issue.as_str(), "issue"); + assert_eq!(SentryResource::MetricAlert.as_str(), "metric_alert"); + assert_eq!(SentryResource::Error.as_str(), "error"); + assert_eq!(SentryResource::Comment.as_str(), "comment"); + assert_eq!(SentryResource::Seer.as_str(), "seer"); + } + #[tokio::test] async fn invalid_action_token_returns_400() { let _guard = tracing_guard();