fix(sentry): redact sensitive request header values in captured events (#2432)

fix(sentry): redact sensitive request header values in captured events

Author: taddes

syncserver-common/src/middleware/sentry.rs

@@ -173,6 +173,46 @@ where
     builder.send();
 }
 
+/// Request headers whose values may contain credentials or user-identifying
+/// data and so must not be recorded on Sentry events. Compared case-insensitively.
+const REDACTED_HEADERS: &[&str] = &[
+    "authorization",
+    "proxy-authorization",
+    "cookie",
+    "x-client-state",
+    "x-keyid",
+    "x-forwarded-for",
+];
+
+/// Placeholder substituted for the value of a [`REDACTED_HEADERS`] header.
+const REDACTED_HEADER_VALUE: &str = "[redacted]";
+
+/// Return the header value to record on a Sentry event, substituting a
+/// placeholder for headers whose values may carry credentials or
+/// user-identifying data.
+fn sentry_header_value(name: &str, value: &str) -> String {
+    if REDACTED_HEADERS.contains(&name.to_ascii_lowercase().as_str()) {
+        REDACTED_HEADER_VALUE.to_owned()
+    } else {
+        value.to_owned()
+    }
+}
+
+/// Redact sensitive request-header values on a Sentry event in place.
+///
+/// Intended to be called from a `before_send` hook so that every event is
+/// scrubbed before transmission — regardless of where it originates — not only
+/// requests built by [`sentry_request_from_http`].
+pub fn scrub_event_request_headers(event: &mut Event<'static>) {
+    if let Some(request) = event.request.as_mut() {
+        for (name, value) in request.headers.iter_mut() {
+            if REDACTED_HEADERS.contains(&name.to_ascii_lowercase().as_str()) {
+                *value = REDACTED_HEADER_VALUE.to_owned();
+            }
+        }
+    }
+}
+
 /// Build a Sentry request struct from the HTTP request
 fn sentry_request_from_http(request: &ServiceRequest) -> sentry::protocol::Request {
     sentry::protocol::Request {
@@ -188,7 +228,13 @@ fn sentry_request_from_http(request: &ServiceRequest) -> sentry::protocol::Reque
         headers: request
             .headers()
             .iter()
-            .map(|(k, v)| (k.to_string(), v.to_str().unwrap_or_default().to_string()))
+            .map(|(k, v)| {
+                let name = k.as_str();
+                (
+                    name.to_owned(),
+                    sentry_header_value(name, v.to_str().unwrap_or_default()),
+                )
+            })
             .collect(),
         ..Default::default()
     }
@@ -344,3 +390,60 @@ fn resolved_backtrace(err: &dyn ReportableError) -> Option<Backtrace> {
     }
     backtrace
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{REDACTED_HEADER_VALUE, scrub_event_request_headers, sentry_header_value};
+
+    #[test]
+    fn before_send_scrub_redacts_request_headers() {
+        use sentry::protocol::{Event, Request};
+
+        let mut headers = std::collections::BTreeMap::new();
+        headers.insert("Authorization".to_owned(), "Bearer secret".to_owned());
+        headers.insert("X-KeyID".to_owned(), "kid-value".to_owned());
+        headers.insert("content-type".to_owned(), "application/json".to_owned());
+
+        let mut event = Event {
+            request: Some(Request {
+                headers,
+                ..Default::default()
+            }),
+            ..Default::default()
+        };
+        scrub_event_request_headers(&mut event);
+
+        let headers = event.request.unwrap().headers;
+        assert_eq!(headers["Authorization"], REDACTED_HEADER_VALUE);
+        assert_eq!(headers["X-KeyID"], REDACTED_HEADER_VALUE);
+        assert_eq!(headers["content-type"], "application/json");
+    }
+
+    #[test]
+    fn sensitive_header_values_are_redacted() {
+        // Sensitive headers are redacted regardless of case.
+        for name in [
+            "authorization",
+            "Authorization",
+            "Cookie",
+            "X-KeyID",
+            "x-client-state",
+            "X-Forwarded-For",
+        ] {
+            assert_eq!(
+                sentry_header_value(name, "sensitive-value"),
+                REDACTED_HEADER_VALUE,
+                "expected `{name}` to be redacted"
+            );
+        }
+    }
+
+    #[test]
+    fn ordinary_header_values_pass_through() {
+        assert_eq!(
+            sentry_header_value("content-type", "application/json"),
+            "application/json"
+        );
+        assert_eq!(sentry_header_value("user-agent", "Firefox"), "Firefox");
+    }
+}

syncserver/src/main.rs

@@ -40,6 +40,15 @@ async fn main() -> Result<(), Box<dyn Error>> {
         // Note: set "debug: true," to diagnose sentry issues
         release: sentry::release_name!(),
         environment: Some(settings.environment.clone().into()),
+        // Scrub sensitive request headers from every event before it is sent,
+        // as a central complement to the per-request filtering in the Sentry
+        // middleware (covers events emitted from any source).
+        before_send: Some(std::sync::Arc::new(
+            |mut event: sentry::protocol::Event<'static>| {
+                syncserver_common::middleware::sentry::scrub_event_request_headers(&mut event);
+                Some(event)
+            },
+        )),
         ..sentry::ClientOptions::default()
     });
     opts.integrations.retain(|i| i.name() != "debug-images");