Skip to content

Commit c080b7c

Browse files
authored
fix(FFESUPPORT-433): do not expose request URL on non-200 response (#410)
1 parent e5f579f commit c080b7c

3 files changed

Lines changed: 121 additions & 6 deletions

File tree

.changeset/lazy-hats-battle.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"eppo_core": patch
3+
"elixir-sdk": patch
4+
"python-sdk": patch
5+
"ruby-sdk": patch
6+
"rust-sdk": patch
7+
---
8+
9+
Sanitize sdkKey from logs on non-200 responses.

eppo_core/src/configuration_fetcher.rs

Lines changed: 110 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ impl ConfigurationFetcher {
7272
],
7373
)
7474
.map_err(|err| {
75-
log::warn!(target: "eppo", "failed to parse flags configuration URL: {err:?}");
75+
log::warn!(target: "eppo", "failed to parse flags configuration URL: {err}");
7676
Error::InvalidBaseUrl(err)
7777
})?;
7878

@@ -85,8 +85,9 @@ impl ConfigurationFetcher {
8585
self.unauthorized = true;
8686
return Error::Unauthorized;
8787
} else {
88-
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {:?}", err);
89-
return Error::from(err);
88+
let err = Error::from(err); // sanitize URL to avoid exposing SDK key
89+
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {err}");
90+
return err;
9091

9192
}
9293
})?;
@@ -112,7 +113,7 @@ impl ConfigurationFetcher {
112113
],
113114
)
114115
.map_err(|err| {
115-
log::warn!(target: "eppo", "failed to parse bandits configuration URL: {err:?}");
116+
log::warn!(target: "eppo", "failed to parse bandits configuration URL: {err}");
116117
Error::InvalidBaseUrl(err)
117118
})?;
118119

@@ -125,8 +126,9 @@ impl ConfigurationFetcher {
125126
self.unauthorized = true;
126127
return Error::Unauthorized;
127128
} else {
128-
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {:?}", err);
129-
return Error::from(err);
129+
let err = Error::from(err); // sanitize URL to avoid exposing SDK key
130+
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {err}");
131+
return err;
130132

131133
}
132134
})?;
@@ -138,3 +140,105 @@ impl ConfigurationFetcher {
138140
Ok(configuration)
139141
}
140142
}
143+
144+
#[cfg(test)]
145+
mod tests {
146+
use super::*;
147+
use crate::{Error, SdkMetadata};
148+
use log::{Level, Log, Metadata, Record};
149+
use std::sync::{Arc, Mutex};
150+
use wiremock::{matchers::method, Mock, MockServer, ResponseTemplate};
151+
152+
// Simple logger that captures log messages
153+
static CAPTURED_LOGS: std::sync::OnceLock<Arc<Mutex<Vec<String>>>> = std::sync::OnceLock::new();
154+
155+
struct TestLogger;
156+
157+
unsafe impl Send for TestLogger {}
158+
unsafe impl Sync for TestLogger {}
159+
160+
impl Log for TestLogger {
161+
fn enabled(&self, metadata: &Metadata) -> bool {
162+
metadata.target() == "eppo"
163+
}
164+
165+
fn log(&self, record: &Record) {
166+
if self.enabled(record.metadata()) {
167+
if let Some(logs) = CAPTURED_LOGS.get() {
168+
let message = format!("{}", record.args());
169+
logs.lock().unwrap().push(message);
170+
}
171+
}
172+
}
173+
174+
fn flush(&self) {}
175+
}
176+
177+
fn setup_test_logger() -> Arc<Mutex<Vec<String>>> {
178+
let logs = Arc::new(Mutex::new(Vec::new()));
179+
CAPTURED_LOGS.set(logs.clone()).ok();
180+
// Try to set logger, ignore error if already set
181+
let _ = log::set_boxed_logger(Box::new(TestLogger));
182+
log::set_max_level(log::LevelFilter::Warn);
183+
logs
184+
}
185+
186+
#[tokio::test]
187+
async fn test_sdk_key_not_exposed_in_error_logs() {
188+
let logs = setup_test_logger();
189+
logs.lock().unwrap().clear();
190+
191+
let test_api_key = "secret-api-key-12345";
192+
193+
// Create a mock server that returns 500 error
194+
let mock_server = MockServer::start().await;
195+
Mock::given(method("GET"))
196+
.respond_with(ResponseTemplate::new(500))
197+
.mount(&mock_server)
198+
.await;
199+
200+
// Create ConfigurationFetcher with the test API key pointing to mock server
201+
let mut fetcher = ConfigurationFetcher::new(ConfigurationFetcherConfig {
202+
base_url: mock_server.uri(),
203+
api_key: test_api_key.to_string(),
204+
sdk_metadata: SdkMetadata {
205+
name: "test-sdk",
206+
version: "1.0.0",
207+
},
208+
});
209+
210+
// Attempt to fetch configuration, which will fail and trigger error logging
211+
let result = fetcher.fetch_configuration().await;
212+
213+
// Verify the request failed
214+
assert!(result.is_err(), "Expected configuration fetch to fail");
215+
216+
// Get captured logs
217+
let captured_logs = logs.lock().unwrap();
218+
let all_logs = captured_logs.join(" ");
219+
220+
// Verify the API key is NOT in any of the log messages
221+
assert!(
222+
!all_logs.contains(test_api_key),
223+
"API key should not appear in log messages. Logs: {}",
224+
all_logs
225+
);
226+
227+
// Also verify the returned error doesn't contain the API key
228+
if let Err(eppo_error) = result {
229+
let error_string = format!("{}", eppo_error);
230+
let error_debug = format!("{:?}", eppo_error);
231+
232+
assert!(
233+
!error_string.contains(test_api_key),
234+
"API key should not appear in error Display: {}",
235+
error_string
236+
);
237+
assert!(
238+
!error_debug.contains(test_api_key),
239+
"API key should not appear in error Debug: {}",
240+
error_debug
241+
);
242+
}
243+
}
244+
}

eppo_core/src/event_ingestion/event_delivery.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ pub(super) enum EventDeliveryError {
2929

3030
impl From<reqwest::Error> for EventDeliveryError {
3131
fn from(err: reqwest::Error) -> Self {
32+
let err = err.without_url(); // sanitize URL to avoid exposing SDK key
33+
3234
if err.is_builder() || err.is_request() {
3335
// Issue with request. Most likely a json serialization error.
3436
EventDeliveryError::NonRetriableError(err)

0 commit comments

Comments
 (0)