fix(okhttp): strip query params from recorded URLs by default to prevent sensitive data leakage

Author: buongarzoni

rollbar-okhttp/README.md

@@ -35,6 +35,7 @@ dependencies {
 NetworkTelemetryRecorder recorder = new NetworkTelemetryRecorder() {
     @Override
     public void recordNetworkEvent(Level level, String method, String url, String statusCode) {
+        // url has query parameters stripped by default (see Security section below)
         rollbar.recordNetworkEventFor(level, method, url, statusCode);
     }
 
@@ -73,3 +74,19 @@ The interceptor will automatically record telemetry events to Rollbar without in
 | Response status `< 400`           | No telemetry recorded, response returned normally       |
 | Response status `>= 400`          | Records a network telemetry event with `Level.CRITICAL` |
 | Connection failure / timeout      | Records an error event, then rethrows the `IOException` |
+
+## Security
+
+URL query parameters often carry sensitive data such as API keys (`?api_key=...`), OAuth tokens (`?access_token=...`), or PII. To prevent accidental leakage to Rollbar, the interceptor **strips query parameters by default** before passing the URL to `NetworkTelemetryRecorder`.
+
+For example, a request to `https://api.example.com/charge?token=sk_live_secret` will be recorded as `https://api.example.com/charge`.
+
+If your URLs do not contain sensitive query parameters and you need them for debugging, you can opt in to the full URL by supplying a custom sanitizer:
+
+```java
+OkHttpClient client = new OkHttpClient.Builder()
+    .addInterceptor(new RollbarOkHttpInterceptor(recorder, HttpUrl::toString))
+    .build();
+```
+
+When using a custom sanitizer, you are responsible for ensuring that sensitive query parameters are removed before the URL reaches Rollbar.

rollbar-okhttp/src/main/java/com/rollbar/okhttp/NetworkTelemetryRecorder.java

@@ -3,6 +3,10 @@
 import com.rollbar.api.payload.data.Level;
 
 public interface NetworkTelemetryRecorder {
+  /**
+   * @param url the request URL with query parameters stripped by default; supply a custom
+   *            sanitizer to {@link RollbarOkHttpInterceptor} to change this behavior.
+   */
   void recordNetworkEvent(Level level, String method, String url, String statusCode);
 
   void recordErrorEvent(Exception exception);

rollbar-okhttp/src/main/java/com/rollbar/okhttp/RollbarOkHttpInterceptor.java

@@ -3,8 +3,11 @@
 import com.rollbar.api.payload.data.Level;
 
 import java.io.IOException;
+import java.util.Objects;
+import java.util.function.Function;
 import java.util.logging.Logger;
 
+import okhttp3.HttpUrl;
 import okhttp3.Interceptor;
 import okhttp3.Request;
 import okhttp3.Response;
@@ -13,10 +16,19 @@ public class RollbarOkHttpInterceptor implements Interceptor {
 
   private static final Logger LOGGER = Logger.getLogger(RollbarOkHttpInterceptor.class.getName());
 
+  private static final Function<HttpUrl, String> DEFAULT_URL_SANITIZER =
+      url -> url.newBuilder().query(null).build().toString();
+
   private final NetworkTelemetryRecorder recorder;
+  private final Function<HttpUrl, String> urlSanitizer;
 
   public RollbarOkHttpInterceptor(NetworkTelemetryRecorder recorder) {
+    this(recorder, DEFAULT_URL_SANITIZER);
+  }
+
+  public RollbarOkHttpInterceptor(NetworkTelemetryRecorder recorder, Function<HttpUrl, String> urlSanitizer) {
     this.recorder = recorder;
+    this.urlSanitizer = Objects.requireNonNull(urlSanitizer, "urlSanitizer must not be null");
   }
 
   @Override
@@ -31,7 +43,7 @@ public Response intercept(Chain chain) throws IOException {
           recorder.recordNetworkEvent(
               Level.CRITICAL,
               request.method(),
-              request.url().toString(),
+              urlSanitizer.apply(request.url()),
               String.valueOf(response.code()));
         } catch (Exception recorderException) {
           LOGGER.log(java.util.logging.Level.WARNING,

rollbar-okhttp/src/test/java/com/rollbar/okhttp/RollbarOkHttpInterceptorTest.java

@@ -14,6 +14,7 @@
 import java.io.IOException;
 
 import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.*;
 
 class RollbarOkHttpInterceptorTest {
@@ -174,4 +175,37 @@ void recorderThrowsOnConnectionFailure_originalIOExceptionPropagates() {
         assertThrows(IOException.class, () -> client.newCall(request).execute());
         verify(recorder).recordErrorEvent(any(IOException.class));
     }
+
+    @Test
+    void defaultSanitizer_stripsQueryParamsFromUrl() throws IOException {
+        server.enqueue(new MockResponse().setResponseCode(500));
+
+        Request request = new Request.Builder()
+                .url(server.url("/sensitive?token=sk_live_secret&email=user@example.com"))
+                .build();
+        Response response = client.newCall(request).execute();
+        response.close();
+
+        verify(recorder).recordNetworkEvent(
+                eq(Level.CRITICAL), eq("GET"),
+                argThat(url -> url.contains("/sensitive") && !url.contains("secret") && !url.contains("email")),
+                eq("500"));
+    }
+
+    @Test
+    void customSanitizer_isAppliedToUrl() throws IOException {
+        server.enqueue(new MockResponse().setResponseCode(500));
+
+        OkHttpClient customClient = new OkHttpClient.Builder()
+                .addInterceptor(new RollbarOkHttpInterceptor(recorder, url -> "Updated String"))
+                .build();
+
+        Request request = new Request.Builder()
+                .url(server.url("/path?secret=abc"))
+                .build();
+        Response response = customClient.newCall(request).execute();
+        response.close();
+
+        verify(recorder).recordNetworkEvent(eq(Level.CRITICAL), eq("GET"), eq("Updated String"), eq("500"));
+    }
 }