diff --git a/README.md b/README.md index ee94396..563953c 100644 --- a/README.md +++ b/README.md @@ -381,13 +381,15 @@ matching payment status. ### Verifying webhooks -Every webhook request carries two headers: +Every webhook request carries two headers that together authenticate the event: | Header | Value | |---|---| | `X-StellarGate-Timestamp` | Unix time (seconds) at which the event was signed | | `X-StellarGate-Signature` | Hex HMAC-SHA256 of `"{timestamp}.{raw_body}"`, keyed with your `WEBHOOK_SECRET` | +A third header, `X-StellarGate-Event`, is included as a routing convenience (e.g. to quickly filter events in a load balancer before parsing JSON). **This header is not part of the signed material** — it mirrors the `event` field in the body but can be altered in transit without invalidating the signature. Always verify the signature first, then read the event type from the signed body. + The signature covers the timestamp as well as the body (Stripe-style), so a captured request cannot be replayed indefinitely. To verify: @@ -400,6 +402,9 @@ captured request cannot be replayed indefinitely. To verify: 4. Compute `HMAC_SHA256(WEBHOOK_SECRET, "{t}.{raw_body}")` and hex-encode it. 5. Compare it to `sig` with a **constant-time** equality check. Reject on mismatch. +6. After the signature passes, read the `event` field from the **body** to + determine the event type. Do **not** route on `X-StellarGate-Event` for + security-sensitive logic. Example (Node.js): @@ -418,6 +423,23 @@ function verify(rawBody, headers, secret, toleranceSec = 300) { .digest("hex"); return crypto.timingSafeEqual(Buffer.from(sig), Buffer.from(expected)); } + +// Usage: always read the event type from the verified body, never from the header. +// X-StellarGate-Event is a convenience header only — it is NOT signed. +function handleWebhook(rawBody, headers, secret) { + if (!verify(rawBody, headers, secret)) { + throw new Error("invalid signature"); + } + const payload = JSON.parse(rawBody); + const event = payload.event; // ← authenticated; safe to route on + // const event = headers["x-stellargate-event"]; // ← NOT authenticated; do not use + switch (event) { + case "payment.completed": /* ... */ break; + case "payment.overpaid": /* ... */ break; + case "payment.underpaid": /* ... */ break; + case "payment.expired": /* ... */ break; + } +} ``` ## Project Structure diff --git a/src/api/payments.rs b/src/api/payments.rs index 0d42dbf..369f7e7 100644 --- a/src/api/payments.rs +++ b/src/api/payments.rs @@ -499,6 +499,9 @@ pub async fn redeliver_webhook( .header("Content-Type", "application/json") .header("X-StellarGate-Signature", &signature) .header("X-StellarGate-Timestamp", timestamp.to_string()) + // Convenience header — mirrors the `event` field already present in + // the signed body. NOT covered by the HMAC; receivers must route on + // the authenticated body field, not this header. .header("X-StellarGate-Event", &event) .body(delivery.payload.clone()) .send() diff --git a/src/config.rs b/src/config.rs index 8fc77cc..e5c8e34 100644 --- a/src/config.rs +++ b/src/config.rs @@ -172,7 +172,6 @@ impl Config { webhook_secret, webhook_retry_attempts: parse_env("WEBHOOK_RETRY_ATTEMPTS", 3)?, webhook_retry_delay_ms: parse_env("WEBHOOK_RETRY_DELAY_MS", 5000)?, - webhook_timeout_secs: parse_env("WEBHOOK_TIMEOUT_SECS", 10)?, poll_interval_secs: parse_env("POLL_INTERVAL_SECS", 10)?, payment_ttl_secs: parse_env("PAYMENT_TTL_SECS", 3600)?, rate_limit_requests_per_sec: parse_env("RATE_LIMIT_REQUESTS_PER_SEC", 10)?, diff --git a/src/webhook.rs b/src/webhook.rs index c79a0b6..4a5ef1f 100644 --- a/src/webhook.rs +++ b/src/webhook.rs @@ -7,6 +7,12 @@ //! - `X-StellarGate-Timestamp`: the Unix time (seconds) the event was signed. //! - `X-StellarGate-Signature`: the hex HMAC-SHA256 of `"{timestamp}.{body}"` //! (Stripe-style), keyed with the shared `WEBHOOK_SECRET`. +//! - `X-StellarGate-Event`: a convenience copy of the `event` field from the +//! body, included so receivers can route cheaply before parsing JSON. +//! **This header is not covered by the HMAC signature.** It can be altered +//! in transit without invalidating the signature. Receivers that make +//! security-sensitive decisions MUST route on the `event` field inside the +//! verified JSON body, not on this header. //! //! Binding the signature to the timestamp stops a captured request from being //! replayed indefinitely: a receiver recomputes the signature over the same @@ -27,6 +33,12 @@ type HmacSha256 = Hmac; /// Compute the hex-encoded HMAC-SHA256 signature for a webhook, binding it to /// `timestamp` by signing the Stripe-style payload `"{timestamp}.{body}"`. /// +/// The `body` is the full JSON event payload produced by [`build_payload`], +/// which always includes an `"event"` field. Because the event type is part of +/// the signed body, receivers can rely on it being authentic after verifying +/// the signature — no separate signing of the `X-StellarGate-Event` header is +/// needed or performed. +/// /// Receivers must recompute the signature over the same `"{timestamp}.{body}"` /// string and reject the request if `timestamp` is too far from their own clock /// (see the README), which is what prevents replay of an old, valid signature. @@ -74,6 +86,12 @@ pub fn build_payload(payment: &db::Payment, event: &str, delta: Option<&str>) -> /// recorded in the `webhook_deliveries` table. Errors are logged, never /// propagated — a failed webhook must not roll back a confirmed payment. /// +/// The signed body (`"{timestamp}.{body}"`) already contains the `event` field, +/// so the event type is fully authenticated by the HMAC signature. The +/// `X-StellarGate-Event` header is also included as a routing convenience but +/// is **not** covered by the signature — receivers must not use it for +/// security-sensitive decisions. +/// /// `delta` is the absolute amount difference included in overpaid/underpaid /// events; pass `None` for exact-payment events. pub async fn dispatch(state: &AppState, payment: &db::Payment, event: &str, delta: Option<&str>) { @@ -124,6 +142,9 @@ pub async fn dispatch(state: &AppState, payment: &db::Payment, event: &str, delt .header("Content-Type", "application/json") .header("X-StellarGate-Signature", &signature) .header("X-StellarGate-Timestamp", timestamp.to_string()) + // Convenience header — mirrors the `event` field already present in + // the signed body. NOT covered by the HMAC; receivers must route on + // the authenticated body field, not this header. .header("X-StellarGate-Event", event) .body(body.clone()) .send() @@ -214,4 +235,51 @@ mod tests { let body = b"{\"event\":\"payment.success\"}"; assert_ne!(sign("secret-a", 1, body), sign("secret-b", 1, body)); } + + #[test] + fn build_payload_includes_event_in_signed_body() { + // The `event` field must be present in the JSON body so that after + // signature verification a receiver can read the authenticated event + // type from the body rather than from the unsigned X-StellarGate-Event + // header. This test locks that contract. + let payment = db::Payment { + id: "pay_1".into(), + merchant_id: "merchant_1".into(), + destination_address: "GDESTINATION".into(), + memo: "ABCD1234".into(), + amount: "10".into(), + asset: "XLM".into(), + status: "completed".into(), + tx_hash: Some("txhash".into()), + paid_amount: Some("10".into()), + webhook_url: None, + created_at: "2026-01-01T00:00:00".into(), + updated_at: "2026-01-01T00:00:01".into(), + expires_at: "2026-01-01T01:00:00".into(), + }; + + for event in &[ + "payment.completed", + "payment.overpaid", + "payment.underpaid", + "payment.expired", + ] { + let payload = build_payload(&payment, event, None); + assert_eq!( + payload["event"].as_str(), + Some(*event), + "build_payload must embed the event type in the JSON body \ + so it is covered by the HMAC signature (event={event})" + ); + + // Confirm the serialised bytes contain the event string, i.e. that + // it survives the round-trip through serde_json::to_vec used in dispatch(). + let body = serde_json::to_vec(&payload).unwrap(); + let body_str = String::from_utf8(body).unwrap(); + assert!( + body_str.contains(event), + "serialised body must contain the event string (event={event})" + ); + } + } } diff --git a/tests/webhook_dispatch_tests.rs b/tests/webhook_dispatch_tests.rs index 3e56200..22456ec 100644 --- a/tests/webhook_dispatch_tests.rs +++ b/tests/webhook_dispatch_tests.rs @@ -187,3 +187,80 @@ async fn dispatch_marks_failed_after_exhausting_retries() { assert_eq!(deliveries[0].status, "failed"); assert_eq!(deliveries[0].attempts, 3); } + +#[tokio::test] +async fn event_field_in_body_matches_header_and_is_covered_by_signature() { + /* Security regression test for issue #160. + * + * The X-StellarGate-Event header is NOT covered by the HMAC signature. + * Receivers MUST route on the `event` field inside the verified JSON body, + * not on the header. This test asserts: + * + * 1. The signed body contains the `event` field. + * 2. The header value mirrors the body's event field (i.e. they agree when + * the request has not been tampered with). + * 3. The HMAC signature is computed over the body (which includes `event`), + * so altering the header would not invalidate the signature — confirming + * the header is informational only. + */ + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/hook")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + let cfg = make_config("test-secret", 1); + let state = setup_state(cfg).await; + let payment = create_test_payment(&state, &format!("{}/hook", server.uri())).await; + + webhook::dispatch(&state, &payment, "payment.completed", None).await; + + let received = server.received_requests().await.unwrap(); + assert_eq!(received.len(), 1); + let req = &received[0]; + + // 1. The header is present. + let header_event = req + .headers + .get("X-StellarGate-Event") + .expect("X-StellarGate-Event header must be present") + .to_str() + .unwrap(); + + // 2. The body contains the `event` field. + let body: serde_json::Value = + serde_json::from_slice(&req.body).expect("body must be valid JSON"); + let body_event = body["event"] + .as_str() + .expect("body must contain an `event` field"); + + // 3. Header and body agree (no tampering in this happy-path test). + assert_eq!( + header_event, body_event, + "X-StellarGate-Event header must mirror the body event field" + ); + assert_eq!(body_event, "payment.completed"); + + // 4. The signature is valid over the body (which contains `event`), + // confirming the event type is authenticated through the body, not the header. + let timestamp: i64 = req + .headers + .get("X-StellarGate-Timestamp") + .unwrap() + .to_str() + .unwrap() + .parse() + .unwrap(); + let expected_sig = webhook::sign(&state.config.webhook_secret, timestamp, &req.body); + assert_eq!( + req.headers + .get("X-StellarGate-Signature") + .unwrap() + .to_str() + .unwrap(), + expected_sig, + "signature must be valid over the body (which contains the event field)" + ); +}