Skip to content

refactor: honor send_default_pii in sentry-actix and sentry-tower #771

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

Merged
merged 6 commits into from
Apr 17, 2025
Merged
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
24 changes: 14 additions & 10 deletions sentry-actix/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ use futures_util::future::{ok, Future, Ready};
use futures_util::{FutureExt as _, TryStreamExt as _};

use sentry_core::protocol::{self, ClientSdkPackage, Event, Request};
use sentry_core::utils::is_sensitive_header;
use sentry_core::MaxRequestBodySize;
use sentry_core::{Hub, SentryFutureExt};

Expand Down Expand Up @@ -212,6 +213,7 @@ pub struct SentryMiddleware<S> {

fn should_capture_request_body(
headers: &HeaderMap,
with_pii: bool,
max_request_body_size: MaxRequestBodySize,
) -> bool {
let is_chunked = headers
Expand All @@ -220,15 +222,16 @@ fn should_capture_request_body(
.map(|transfer_encoding| transfer_encoding.contains("chunked"))
.unwrap_or(false);

let is_valid_content_type = headers
.get(header::CONTENT_TYPE)
.and_then(|h| h.to_str().ok())
.is_some_and(|content_type| {
matches!(
content_type,
"application/json" | "application/x-www-form-urlencoded"
)
});
let is_valid_content_type = with_pii
|| headers
.get(header::CONTENT_TYPE)
.and_then(|h| h.to_str().ok())
.is_some_and(|content_type| {
matches!(
content_type,
"application/json" | "application/x-www-form-urlencoded"
)
});

let is_within_size_limit = headers
.get(header::CONTENT_LENGTH)
Expand Down Expand Up @@ -322,7 +325,7 @@ where
async move {
let mut req = req;

if should_capture_request_body(req.headers(), max_request_body_size) {
if should_capture_request_body(req.headers(), with_pii, max_request_body_size) {
sentry_req.data = Some(capture_request_body(&mut req).await);
}

Expand Down Expand Up @@ -424,6 +427,7 @@ fn sentry_request_from_http(request: &ServiceRequest, with_pii: bool) -> Request
.headers()
.iter()
.filter(|(_, v)| !v.is_sensitive())
.filter(|(k, _)| with_pii || !is_sensitive_header(k.as_str()))
.map(|(k, v)| (k.to_string(), v.to_str().unwrap_or_default().to_string()))
.collect(),
..Default::default()
Expand Down
2 changes: 1 addition & 1 deletion sentry-core/src/clientoptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub struct ClientOptions {
pub max_breadcrumbs: usize,
/// Attaches stacktraces to messages.
pub attach_stacktrace: bool,
/// If turned on some default PII informat is attached.
/// If turned on, some information that can be considered PII is captured, such as potentially sensitive HTTP headers and user IP address in HTTP server integrations.
pub send_default_pii: bool,
/// The server name to be reported.
pub server_name: Option<Cow<'static, str>>,
Expand Down
3 changes: 3 additions & 0 deletions sentry-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,6 @@ pub mod test;
pub use sentry_types as types;
pub use sentry_types::protocol::v7 as protocol;
pub use sentry_types::protocol::v7::{Breadcrumb, Envelope, Level, User};

// utilities reused across integrations
pub mod utils;
17 changes: 17 additions & 0 deletions sentry-core/src/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//! Utilities reused across dependant crates and integrations.

const SENSITIVE_HEADERS_UPPERCASE: &[&str] = &[
"AUTHORIZATION",
"PROXY_AUTHORIZATION",
"COOKIE",
"SET_COOKIE",
"X_FORWARDED_FOR",
"X_REAL_IP",
"X_API_KEY",
];

/// Determines if the HTTP header with the given name shall be considered as potentially carrying
/// sensitive data.
pub fn is_sensitive_header(name: &str) -> bool {
SENSITIVE_HEADERS_UPPERCASE.contains(&name.to_ascii_uppercase().replace("-", "_").as_str())
}
45 changes: 38 additions & 7 deletions sentry-tower/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ use std::task::{Context, Poll};

use http::{header, uri, Request, Response, StatusCode};
use pin_project::pinned_drop;
use sentry_core::protocol;
use sentry_core::utils::is_sensitive_header;
use sentry_core::{protocol, Hub};
use tower_layer::Layer;
use tower_service::Service;

/// Tower Layer that logs Http Request Headers.
/// Tower Layer that captures Http Request information.
///
/// The Service created by this Layer can also optionally start a new
/// The Service created by this Layer can optionally start a new
/// performance monitoring transaction for each incoming request,
/// continuing the trace based on incoming distributed tracing headers.
///
Expand All @@ -20,35 +21,63 @@ use tower_service::Service;
/// or similar. In this case, users should manually override the transaction name
/// in the request handler using the [`Scope::set_transaction`](sentry_core::Scope::set_transaction)
/// method.
///
/// By default, the service will filter out potentially sensitive headers from the captured
/// requests. By enabling `with_pii`, you can opt in to capturing all headers instead.
#[derive(Clone, Default)]
pub struct SentryHttpLayer {
start_transaction: bool,
with_pii: bool,
}

impl SentryHttpLayer {
/// Creates a new Layer that only logs Request Headers.
/// Creates a new Layer that only captures request information.
/// If a client is bound to the main Hub (i.e. the SDK has already been initialized), set `with_pii` based on the `send_default_pii` client option.
pub fn new() -> Self {
Self::default()
let mut slf = Self::default();
Hub::main()
.client()
.inspect(|client| slf.with_pii = client.options().send_default_pii);
slf
}

/// Creates a new Layer which starts a new performance monitoring transaction
/// for each incoming request.
#[deprecated(since = "0.38.0", note = "please use `enable_transaction` instead")]
pub fn with_transaction() -> Self {
Self {
start_transaction: true,
with_pii: false,
}
}

/// Enable starting a new performance monitoring transaction for each incoming request.
#[must_use]
pub fn enable_transaction(mut self) -> Self {
self.start_transaction = true;
self
}

/// Include PII in captured requests. Potentially sensitive headers are not filtered out.
#[must_use]
pub fn enable_pii(mut self) -> Self {
self.with_pii = true;
self
}
}

/// Tower Service that logs Http Request Headers.
/// Tower Service that captures Http Request information.
///
/// The Service can also optionally start a new performance monitoring transaction
/// The Service can optionally start a new performance monitoring transaction
/// for each incoming request, continuing the trace based on incoming
/// distributed tracing headers.
///
/// If `with_pii` is disabled, sensitive headers will be filtered out.
#[derive(Clone)]
pub struct SentryHttpService<S> {
service: S,
start_transaction: bool,
with_pii: bool,
}

impl<S> Layer<S> for SentryHttpLayer {
Expand All @@ -58,6 +87,7 @@ impl<S> Layer<S> for SentryHttpLayer {
Self::Service {
service,
start_transaction: self.start_transaction,
with_pii: self.with_pii,
}
}
}
Expand Down Expand Up @@ -161,6 +191,7 @@ where
.headers()
.into_iter()
.filter(|(_, value)| !value.is_sensitive())
.filter(|(header, _)| self.with_pii || !is_sensitive_header(header.as_str()))
.map(|(header, value)| {
(
header.to_string(),
Expand Down
2 changes: 1 addition & 1 deletion sentry-tower/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
//! # type Request = http::Request<String>;
//! let layer = tower::ServiceBuilder::new()
//! .layer(sentry_tower::NewSentryLayer::<Request>::new_from_top())
//! .layer(sentry_tower::SentryHttpLayer::with_transaction());
//! .layer(sentry_tower::SentryHttpLayer::new().enable_transaction());
//! # }
//! ```
//!
Expand Down