diff --git a/rollbar-api/src/main/java/com/rollbar/api/scrubbing/DefaultUrlSanitizer.java b/rollbar-api/src/main/java/com/rollbar/api/scrubbing/DefaultUrlSanitizer.java new file mode 100644 index 00000000..1329db97 --- /dev/null +++ b/rollbar-api/src/main/java/com/rollbar/api/scrubbing/DefaultUrlSanitizer.java @@ -0,0 +1,51 @@ +package com.rollbar.api.scrubbing; + +/** + * Default {@link StringUrlSanitizer} that strips userinfo, query string, and fragment from URLs. + * Uses string scanning rather than {@code java.net.URI} to avoid allocation on clean URLs + * and to preserve the original percent-encoding without normalization. + */ +public final class DefaultUrlSanitizer implements StringUrlSanitizer { + + public static final DefaultUrlSanitizer INSTANCE = new DefaultUrlSanitizer(); + + private DefaultUrlSanitizer() { + } + + @Override + public String sanitize(String url) { + if (url == null) { + return null; + } + // Fast path: no characters that can introduce query string, fragment, or userinfo. + if (url.indexOf('?') < 0 && url.indexOf('#') < 0 && url.indexOf('@') < 0) { + return url; + } + return strip(url); + } + + private static String strip(String url) { + int end = url.length(); + int q = url.indexOf('?'); + int f = url.indexOf('#'); + if (q >= 0 && q < end) { + end = q; + } + if (f >= 0 && f < end) { + end = f; + } + // Strip userinfo: find "://" then the last "@" before the first "/" after the authority start. + String result = url.substring(0, end); + int schemeEnd = result.indexOf("://"); + if (schemeEnd >= 0) { + int hostStart = schemeEnd + 3; + int slashAfterHost = result.indexOf('/', hostStart); + int searchEnd = slashAfterHost < 0 ? result.length() : slashAfterHost; + int at = result.lastIndexOf('@', searchEnd); + if (at >= hostStart) { + result = result.substring(0, hostStart) + result.substring(at + 1); + } + } + return result; + } +} diff --git a/rollbar-api/src/main/java/com/rollbar/api/scrubbing/StringUrlSanitizer.java b/rollbar-api/src/main/java/com/rollbar/api/scrubbing/StringUrlSanitizer.java new file mode 100644 index 00000000..f526627f --- /dev/null +++ b/rollbar-api/src/main/java/com/rollbar/api/scrubbing/StringUrlSanitizer.java @@ -0,0 +1,18 @@ +package com.rollbar.api.scrubbing; + +/** + * Sanitizes a URL string before it is included in a Rollbar payload. + * Implementations should strip sensitive components such as userinfo, + * query parameters, and fragments. + */ +@FunctionalInterface +public interface StringUrlSanitizer { + /** + * Returns a sanitized version of the given URL string, or {@code null} if + * the input is {@code null}. + * + * @param url the raw URL string, may be {@code null}. + * @return the sanitized URL, or {@code null}. + */ + String sanitize(String url); +} diff --git a/rollbar-api/src/test/java/com/rollbar/api/scrubbing/DefaultUrlSanitizerTest.java b/rollbar-api/src/test/java/com/rollbar/api/scrubbing/DefaultUrlSanitizerTest.java new file mode 100644 index 00000000..9ea44eda --- /dev/null +++ b/rollbar-api/src/test/java/com/rollbar/api/scrubbing/DefaultUrlSanitizerTest.java @@ -0,0 +1,96 @@ +package com.rollbar.api.scrubbing; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class DefaultUrlSanitizerTest { + + private final DefaultUrlSanitizer sanitizer = DefaultUrlSanitizer.INSTANCE; + + @Test + public void nullInputReturnsNull() { + assertNull(sanitizer.sanitize(null)); + } + + @Test + public void cleanUrlUnchanged() { + String url = "https://example.com/api/v1/things"; + assertEquals(url, sanitizer.sanitize(url)); + } + + @Test + public void queryStringStripped() { + assertEquals( + "https://example.com/search", + sanitizer.sanitize("https://example.com/search?token=abc&page=1") + ); + } + + @Test + public void fragmentStripped() { + assertEquals( + "https://example.com/page", + sanitizer.sanitize("https://example.com/page#section") + ); + } + + @Test + public void userinfoStripped() { + assertEquals( + "https://example.com/path", + sanitizer.sanitize("https://user:pass@example.com/path") + ); + } + + @Test + public void allThreeScrubbed() { + assertEquals( + "https://example.com/path", + sanitizer.sanitize("https://admin:secret@example.com/path?token=xyz#top") + ); + } + + @Test + public void malformedUrlNoException() { + // Should not throw; best-effort strip + String result = sanitizer.sanitize("not-a-url?query=sensitive"); + assertNotNull(result); + assertFalse(result.contains("sensitive")); + } + + @Test + public void malformedUrlWithUserinfo() { + String result = sanitizer.sanitize("http://user:secret@host/path?q=1"); + assertNotNull(result); + assertFalse(result.contains("secret")); + assertFalse(result.contains("q=1")); + } + + @Test + public void emptyStringUnchanged() { + assertEquals("", sanitizer.sanitize("")); + } + + @Test + public void cleanUrlReturnedAsSameInstance() { + String url = "https://example.com/api/v1/things"; + assertSame(url, sanitizer.sanitize(url)); + } + + @Test + public void percentEncodedPathPreserved() { + // No ?, #, or @ — fast path must return the same instance without normalizing encoding. + String url = "https://example.com/path%20with%20spaces"; + assertSame(url, sanitizer.sanitize(url)); + } + + @Test + public void atSignInPathNotTreatedAsUserinfo() { + // The @ is after the first path slash, so it is not userinfo. + String url = "https://example.com/users/@alice?token=x"; + String result = sanitizer.sanitize(url); + assertTrue(result.contains("@alice")); + assertFalse(result.contains("token")); + } +} diff --git a/rollbar-java/src/integTest/java/com/rollbar/notifier/ScrubbingITest.java b/rollbar-java/src/integTest/java/com/rollbar/notifier/ScrubbingITest.java new file mode 100644 index 00000000..d3ba637c --- /dev/null +++ b/rollbar-java/src/integTest/java/com/rollbar/notifier/ScrubbingITest.java @@ -0,0 +1,203 @@ +package com.rollbar.notifier; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static com.rollbar.notifier.config.ConfigBuilder.withAccessToken; +import static java.lang.String.format; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import com.google.gson.Gson; +import com.rollbar.api.payload.data.Data; +import com.rollbar.api.payload.data.Level; +import com.rollbar.notifier.config.Config; +import com.rollbar.notifier.config.ConfigBuilder; +import com.rollbar.notifier.scrubbing.ScrubDataTransformer; +import com.rollbar.notifier.sender.Sender; +import com.rollbar.notifier.sender.SyncSender; +import com.rollbar.notifier.transformer.Transformer; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.UUID; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +/** + * End-to-end coverage of the built-in scrubber: it must run after any user transformer, follow + * reconfiguration, reach data nested inside collections, and keep network telemetry URLs clean. + * Assertions are made against the JSON WireMock actually received, so the whole serialization + * path is exercised. + */ +public class ScrubbingITest { + + private static final String ACCESS_TOKEN = UUID.randomUUID().toString(); + + private static final String SCRUBBED = ScrubDataTransformer.SCRUBBED_VALUE; + + @Rule + public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().dynamicPort()); + + private Sender sender; + + private ConfigBuilder configBuilder; + + @Before + public void setUp() { + this.sender = buildSender(getUrl()); + this.configBuilder = withAccessToken(ACCESS_TOKEN).sender(sender); + + stubFor(post(urlEqualTo("/api/1/item/")) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"err\":0,\"result\":{\"uuid\":\"" + UUID.randomUUID() + "\"}}"))); + } + + @After + public void tearDown() throws Exception { + this.sender.close(true); + } + + @Test + public void builtInScrubbingRunsAfterTheUserTransformer() { + // The user transformer injects the secret, so it can only be redacted if the built-in + // scrubber runs afterwards. + Transformer injectSecret = data -> new Data.Builder(data) + .custom(objectMap("password", "hunter2", "user", "alice")) + .build(); + + Config config = configBuilder + .transformer(injectSecret) + .redactedKeys(Collections.singletonList("password")) + .build(); + + new Rollbar(config).error("boom"); + + Map custom = getValue(sentData(0), "custom"); + assertThat(custom.get("password"), is(SCRUBBED)); + assertThat(custom.get("user"), is("alice")); + } + + @Test + public void reconfigurationChangesTheRedactedKeys() { + Rollbar rollbar = new Rollbar(configBuilder + .redactedKeys(Collections.singletonList("password")) + .build()); + + rollbar.error("boom", objectMap("password", "hunter2", "token", "secret-token")); + + Map before = getValue(sentData(0), "custom"); + assertThat(before.get("password"), is(SCRUBBED)); + assertThat(before.get("token"), is("secret-token")); + + rollbar.configure(configBuilder + .redactedKeys(Collections.singletonList("token")) + .build()); + + rollbar.error("boom", objectMap("password", "hunter2", "token", "secret-token")); + + Map after = getValue(sentData(1), "custom"); + assertThat(after.get("password"), is("hunter2")); + assertThat(after.get("token"), is(SCRUBBED)); + } + + @Test + public void nestedCollectionsAreScrubbedEndToEnd() { + Config config = configBuilder + .redactedKeys(Collections.singletonList("password")) + .build(); + + Map custom = new HashMap<>(); + custom.put("users", Arrays.asList(objectMap("password", "hunter2"), objectMap("name", "bob"))); + custom.put("keys", new Object[] {objectMap("password", "hunter2")}); + + new Rollbar(config).error("boom", custom); + + Map sentCustom = getValue(sentData(0), "custom"); + + List> users = getValue(sentCustom, "users"); + assertThat(users, hasSize(2)); + assertThat(users.get(0).get("password"), is(SCRUBBED)); + assertThat(users.get(1).get("name"), is("bob")); + + List> keys = getValue(sentCustom, "keys"); + assertThat(keys, hasSize(1)); + assertThat(keys.get(0).get("password"), is(SCRUBBED)); + } + + @Test + public void networkTelemetryUrlsAreSanitized() { + Rollbar rollbar = new Rollbar(configBuilder.build()); + + rollbar.recordNetworkEventFor(Level.CRITICAL, "GET", + "https://user:pass@example.com/orders?token=secret#frag", "500"); + rollbar.error("boom"); + + List> telemetry = getValue(sentData(0), "body", "telemetry"); + assertThat(telemetry, hasSize(1)); + Map body = getValue(telemetry.get(0), "body"); + assertThat(body.get("url"), is("https://example.com/orders")); + } + + // --- helpers --- + + protected Sender buildSender(String url) { + return new SyncSender.Builder().url(url).accessToken(ScrubbingITest.ACCESS_TOKEN).build(); + } + + /** The parsed {@code data} object of the nth payload WireMock received. */ + @SuppressWarnings("unchecked") + private Map sentData(int index) { + List requests = + WireMock.findAll(postRequestedFor(urlEqualTo("/api/1/item/"))); + Map payload = + new Gson().fromJson(requests.get(index).getBodyAsString(), Map.class); + return getValue(payload, "data"); + } + + @SuppressWarnings("unchecked") + private static T getValue(Map source, String attribute, + String... attributes) { + Object value = source.get(attribute); + + if (attributes.length == 0) { + return (T) value; + } + + if (value == null) { + throw new NullPointerException("No value with key " + attribute); + } + + Map asMap = (Map) value; + String[] newAttributes = new String[attributes.length - 1]; + System.arraycopy(attributes, 1, newAttributes, 0, newAttributes.length); + + return getValue(asMap, attributes[0], newAttributes); + } + + private static Map objectMap(String... kvPairs) { + Map map = new HashMap<>(); + for (int i = 0; i < kvPairs.length; i += 2) { + map.put(kvPairs[i], kvPairs[i + 1]); + } + return map; + } + + private String getUrl() { + return format(Locale.US, "http://localhost:%d/api/1/item/", wireMockRule.port()); + } +} diff --git a/rollbar-java/src/main/java/com/rollbar/notifier/RollbarBase.java b/rollbar-java/src/main/java/com/rollbar/notifier/RollbarBase.java index ca5f45ec..8f53a9f3 100644 --- a/rollbar-java/src/main/java/com/rollbar/notifier/RollbarBase.java +++ b/rollbar-java/src/main/java/com/rollbar/notifier/RollbarBase.java @@ -8,8 +8,11 @@ import com.rollbar.api.payload.data.TelemetryEvent; import com.rollbar.api.payload.data.TelemetryType; import com.rollbar.api.payload.data.body.Body; +import com.rollbar.api.scrubbing.DefaultUrlSanitizer; +import com.rollbar.api.scrubbing.StringUrlSanitizer; import com.rollbar.jvmti.ThrowableCache; import com.rollbar.notifier.config.CommonConfig; +import com.rollbar.notifier.scrubbing.ScrubDataTransformer; import com.rollbar.notifier.telemetry.TelemetryEventTracker; import com.rollbar.notifier.truncation.PayloadTruncator; import com.rollbar.notifier.util.BodyFactory; @@ -43,6 +46,13 @@ public abstract class RollbarBase { protected C config; + private volatile ScrubDataTransformer builtInScrubber; + + // Network telemetry URLs are sanitized when they are recorded rather than when the payload is + // built: TelemetryEvent is opaque once constructed, and an event is recorded once but can be + // attached to many payloads. + private volatile StringUrlSanitizer telemetryUrlSanitizer; + protected final ReadWriteLock configReadWriteLock = new ReentrantReadWriteLock(); protected final Lock configReadLock = configReadWriteLock.readLock(); protected final Lock configWriteLock = configReadWriteLock.writeLock(); @@ -55,6 +65,8 @@ protected RollbarBase(C config, BodyFactory bodyFactory, RESULT emptyResult) { this.bodyFactory = bodyFactory; this.emptyResult = emptyResult; this.telemetryEventTracker = config.telemetryEventTracker(); + this.builtInScrubber = new ScrubDataTransformer(config.redactedKeys(), config.urlSanitizer()); + this.telemetryUrlSanitizer = urlSanitizerOf(config); } /** @@ -93,6 +105,12 @@ public void recordNavigationEventFor(Level level, String from, String to) { * Record network telemetry event with method, url, and status code. * ({@link TelemetryType#NETWORK}). * + *

The url is sanitized with the configured + * {@link CommonConfig#urlSanitizer() url sanitizer} before it is recorded, so credentials and + * query strings do not reach Rollbar. Callers that record through + * {@link #getTelemetryEventTracker()} directly bypass this and are responsible for sanitizing + * themselves. + * * @param level the TelemetryEvent severity (e.g. {@link Level#DEBUG}). * @param method the verb used (e.g. "POST"). * @param url the api url (e.g. " @@ -100,7 +118,10 @@ public void recordNavigationEventFor(Level level, String from, String to) { * @param statusCode the response status code (e.g. "404"). */ public void recordNetworkEventFor(Level level, String method, String url, String statusCode) { - telemetryEventTracker.recordNetworkEventFor(level, getSource(), method, url, statusCode); + StringUrlSanitizer sanitizer = this.telemetryUrlSanitizer; + String sanitizedUrl = url != null ? sanitizer.sanitize(url) : null; + telemetryEventTracker.recordNetworkEventFor(level, getSource(), method, sanitizedUrl, + statusCode); } /** @@ -123,11 +144,23 @@ protected void configure(C config) { this.config = config; configureTruncation(config); processAppPackages(config); + this.builtInScrubber = new ScrubDataTransformer(config.redactedKeys(), config.urlSanitizer()); + this.telemetryUrlSanitizer = urlSanitizerOf(config); } finally { this.configWriteLock.unlock(); } } + /** + * {@link CommonConfig#urlSanitizer()} is documented as never {@code null}, but it is a default + * method a third-party implementation can override, so fall back the same way + * {@link ScrubDataTransformer} does. + */ + private static StringUrlSanitizer urlSanitizerOf(CommonConfig config) { + StringUrlSanitizer sanitizer = config.urlSanitizer(); + return sanitizer != null ? sanitizer : DefaultUrlSanitizer.INSTANCE; + } + private void configureTruncation(C config) { if (config.truncateLargePayloads()) { ObjectsUtils.requireNonNull(config.jsonSerializer(), @@ -250,10 +283,12 @@ protected Data buildData(CommonConfig config, ThrowableWrapper error, Map custom, String description, Level level, boolean isUncaught) { C config; + ScrubDataTransformer scrubber; this.configReadLock.lock(); try { config = this.config; + scrubber = this.builtInScrubber; } finally { this.configReadLock.unlock(); } @@ -280,6 +315,10 @@ protected RESULT process(ThrowableWrapper error, Map custom, Str data = config.transformer().transform(data); } + // Built-in scrubbing always runs after the user transformer + LOGGER.debug("Applying built-in scrubber."); + data = scrubber.transform(data); + // Append if needed uuid or fingerprint data. if (config.uuidGenerator() != null || config.fingerPrintGenerator() != null) { Data.Builder dataBuilder = new Data.Builder(data); diff --git a/rollbar-java/src/main/java/com/rollbar/notifier/config/CommonConfig.java b/rollbar-java/src/main/java/com/rollbar/notifier/config/CommonConfig.java index c4c4ea41..f59d9965 100644 --- a/rollbar-java/src/main/java/com/rollbar/notifier/config/CommonConfig.java +++ b/rollbar-java/src/main/java/com/rollbar/notifier/config/CommonConfig.java @@ -6,6 +6,8 @@ import com.rollbar.api.payload.data.Person; import com.rollbar.api.payload.data.Request; import com.rollbar.api.payload.data.Server; +import com.rollbar.api.scrubbing.DefaultUrlSanitizer; +import com.rollbar.api.scrubbing.StringUrlSanitizer; import com.rollbar.notifier.filter.Filter; import com.rollbar.notifier.fingerprint.FingerprintGenerator; import com.rollbar.notifier.provider.Provider; @@ -13,6 +15,7 @@ import com.rollbar.notifier.telemetry.TelemetryEventTracker; import com.rollbar.notifier.transformer.Transformer; import com.rollbar.notifier.uuid.UuidGenerator; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -223,6 +226,29 @@ default boolean compressPayload() { return true; } + /** + * Keys (matched as case-insensitive regex) whose values should be redacted in headers, + * query/POST parameters, custom data, and {@code Frame.locals} before sending to Rollbar. + * The default header deny-list (Authorization, Cookie, etc.) is always applied regardless + * of this list. + * + * @return list of regex patterns; empty list by default. + */ + default List redactedKeys() { + return Collections.emptyList(); + } + + /** + * URL sanitizer applied to {@link com.rollbar.api.payload.data.Request#getUrl()} before the + * payload is sent. Defaults to {@link DefaultUrlSanitizer#INSTANCE} which strips userinfo, + * query string, and fragment. + * + * @return the URL sanitizer; never {@code null}. + */ + default StringUrlSanitizer urlSanitizer() { + return DefaultUrlSanitizer.INSTANCE; + } + int maximumTelemetryData(); TelemetryEventTracker telemetryEventTracker(); diff --git a/rollbar-java/src/main/java/com/rollbar/notifier/config/ConfigBuilder.java b/rollbar-java/src/main/java/com/rollbar/notifier/config/ConfigBuilder.java index d91e3705..0ff59a44 100644 --- a/rollbar-java/src/main/java/com/rollbar/notifier/config/ConfigBuilder.java +++ b/rollbar-java/src/main/java/com/rollbar/notifier/config/ConfigBuilder.java @@ -6,6 +6,8 @@ import com.rollbar.api.payload.data.Person; import com.rollbar.api.payload.data.Request; import com.rollbar.api.payload.data.Server; +import com.rollbar.api.scrubbing.DefaultUrlSanitizer; +import com.rollbar.api.scrubbing.StringUrlSanitizer; import com.rollbar.notifier.Rollbar; import com.rollbar.notifier.filter.Filter; import com.rollbar.notifier.fingerprint.FingerprintGenerator; @@ -89,6 +91,10 @@ public class ConfigBuilder { protected boolean compressPayload; + protected List redactedKeys; + + protected StringUrlSanitizer urlSanitizer; + private int maximumTelemetryData = RollbarTelemetryEventTracker.MAXIMUM_CAPACITY_FOR_TELEMETRY_EVENTS; @@ -142,6 +148,8 @@ private ConfigBuilder(Config config) { this.compressPayload = config.compressPayload(); this.maximumTelemetryData = config.maximumTelemetryData(); this.telemetryEventTracker = config.telemetryEventTracker(); + this.redactedKeys = config.redactedKeys(); + this.urlSanitizer = config.urlSanitizer(); } /** @@ -523,6 +531,31 @@ public ConfigBuilder telemetryEventTracker(TelemetryEventTracker telemetryEventT return this; } + /** + * Keys (matched as case-insensitive regex) whose values will be redacted in request headers, + * query/POST parameters, custom data, and {@code Frame.locals}. These are additive to the + * built-in header deny-list (Authorization, Cookie, etc.). + * + * @param redactedKeys list of regex patterns. + * @return the builder instance. + */ + public ConfigBuilder redactedKeys(List redactedKeys) { + this.redactedKeys = redactedKeys; + return this; + } + + /** + * URL sanitizer applied to the request URL before the payload is sent. + * Defaults to {@link DefaultUrlSanitizer#INSTANCE}. + * + * @param urlSanitizer the sanitizer. + * @return the builder instance. + */ + public ConfigBuilder urlSanitizer(StringUrlSanitizer urlSanitizer) { + this.urlSanitizer = urlSanitizer; + return this; + } + /** * Builds the {@link Config config}. * @@ -624,6 +657,10 @@ private static class ConfigImpl implements Config { private final TelemetryEventTracker telemetryEventTracker; + private final List redactedKeys; + + private final StringUrlSanitizer urlSanitizer; + ConfigImpl(ConfigBuilder builder) { this.accessToken = builder.accessToken; this.endpoint = builder.endpoint; @@ -659,6 +696,10 @@ private static class ConfigImpl implements Config { this.compressPayload = builder.compressPayload; this.maximumTelemetryData = builder.maximumTelemetryData; this.telemetryEventTracker = builder.telemetryEventTracker; + this.redactedKeys = builder.redactedKeys != null + ? builder.redactedKeys : Collections.emptyList(); + this.urlSanitizer = builder.urlSanitizer != null + ? builder.urlSanitizer : DefaultUrlSanitizer.INSTANCE; } @Override @@ -820,5 +861,15 @@ public int maximumTelemetryData() { public TelemetryEventTracker telemetryEventTracker() { return this.telemetryEventTracker; } + + @Override + public List redactedKeys() { + return redactedKeys; + } + + @Override + public StringUrlSanitizer urlSanitizer() { + return urlSanitizer; + } } } diff --git a/rollbar-java/src/main/java/com/rollbar/notifier/scrubbing/ScrubDataTransformer.java b/rollbar-java/src/main/java/com/rollbar/notifier/scrubbing/ScrubDataTransformer.java new file mode 100644 index 00000000..4ba6dac7 --- /dev/null +++ b/rollbar-java/src/main/java/com/rollbar/notifier/scrubbing/ScrubDataTransformer.java @@ -0,0 +1,503 @@ +package com.rollbar.notifier.scrubbing; + +import com.rollbar.api.payload.data.Data; +import com.rollbar.api.payload.data.Request; +import com.rollbar.api.payload.data.body.Body; +import com.rollbar.api.payload.data.body.BodyContent; +import com.rollbar.api.payload.data.body.Frame; +import com.rollbar.api.payload.data.body.Group; +import com.rollbar.api.payload.data.body.RollbarThread; +import com.rollbar.api.payload.data.body.Trace; +import com.rollbar.api.payload.data.body.TraceChain; +import com.rollbar.api.scrubbing.DefaultUrlSanitizer; +import com.rollbar.api.scrubbing.StringUrlSanitizer; +import com.rollbar.notifier.transformer.Transformer; + +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.regex.Pattern; + +/** + * Built-in {@link Transformer} that scrubs sensitive values from payloads before they are sent + * to Rollbar. Applied automatically after any user-provided transformer. + * + *

By default the following request headers are redacted: + * {@code Authorization}, {@code Cookie}, {@code Set-Cookie}, {@code X-Api-Key}, + * {@code X-Auth-Token}, {@code X-Access-Token}, {@code X-Secret}, + * {@code Proxy-Authorization}, {@code WWW-Authenticate}. + * + *

Additional keys can be configured via {@code ConfigBuilder.redactedKeys(List)}. They are + * matched as case-insensitive regexes against header names, routing parameter keys + * ({@code Request.params}), query and POST parameter keys, request metadata keys + * ({@code Request.metadata}), custom data keys, and {@code Frame.locals} keys. + * {@code Frame.locals} are scrubbed both in the top-level body content and in the trace chains + * carried by {@code Body.rollbarThreads}. + * + *

Nested data is walked recursively: maps reachable through other maps, through + * {@link Collection}s and through object arrays are all scrubbed, up to 8 levels of nesting. The + * surrounding shape is preserved, so a list stays a list and an array stays an array. + * + *

The built-in header deny-list above applies to {@code Request.headers} only; every other + * slot matches on the configured keys alone. + */ +public final class ScrubDataTransformer implements Transformer { + + public static final String SCRUBBED_VALUE = "***"; + + // O(1) set lookup; avoids Matcher allocation on every header key. + private static final Set DEFAULT_HEADERS = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList( + "authorization", "cookie", "set-cookie", "x-api-key", "x-auth-token", + "x-access-token", "x-secret", "proxy-authorization", "www-authenticate" + )) + ); + + // Recursion cap for nested containers in custom data, request payloads and Frame.locals. Every + // map, collection or array counts as one level; this also terminates cyclic structures. + private static final int MAX_SCRUB_DEPTH = 8; + + private final List fieldPatterns; + private final StringUrlSanitizer urlSanitizer; + + /** + * Constructor. + * + * @param redactedKeys keys to redact, matched as case-insensitive regexes. May be {@code null} + * or empty, in which case only the built-in header deny-list and the URL sanitizer apply. + * @param urlSanitizer sanitizer applied to the request URL. Falls back to + * {@link DefaultUrlSanitizer#INSTANCE} when {@code null}. + */ + public ScrubDataTransformer(List redactedKeys, StringUrlSanitizer urlSanitizer) { + this.urlSanitizer = urlSanitizer != null ? urlSanitizer : DefaultUrlSanitizer.INSTANCE; + if (redactedKeys == null || redactedKeys.isEmpty()) { + this.fieldPatterns = Collections.emptyList(); + } else { + List patterns = new ArrayList<>(redactedKeys.size()); + for (String key : redactedKeys) { + patterns.add(Pattern.compile(key, Pattern.CASE_INSENSITIVE)); + } + this.fieldPatterns = Collections.unmodifiableList(patterns); + } + } + + @Override + public Data transform(Data data) { + if (data == null) { + return null; + } + + Request originalRequest = data.getRequest(); + Map originalCustom = data.getCustom(); + Body originalBody = data.getBody(); + + Request scrubbedRequest = scrubRequest(originalRequest); + Map scrubbedCustom = scrubObjectMap(originalCustom, fieldPatterns, 0); + Body scrubbedBody = scrubBody(originalBody); + + boolean changed = scrubbedRequest != originalRequest + || scrubbedCustom != originalCustom + || scrubbedBody != originalBody; + + if (!changed) { + return data; + } + + Data.Builder builder = new Data.Builder(data); + if (scrubbedRequest != originalRequest) { + builder.request(scrubbedRequest); + } + if (scrubbedCustom != originalCustom) { + builder.custom(scrubbedCustom); + } + if (scrubbedBody != originalBody) { + builder.body(scrubbedBody); + } + return builder.build(); + } + + private Request scrubRequest(Request req) { + if (req == null) { + return null; + } + + String originalUrl = req.getUrl(); + Map originalHeaders = req.getHeaders(); + Map originalParams = req.getParams(); + Map> originalGet = req.getGet(); + Map originalPost = req.getPost(); + Map originalMetadata = req.getMetadata(); + String originalQueryString = req.getQueryString(); + + String scrubbedUrl = originalUrl != null ? urlSanitizer.sanitize(originalUrl) : null; + Map scrubbedHeaders = scrubHeaders(originalHeaders); + Map scrubbedParams = scrubStringMap(originalParams, fieldPatterns); + Map> scrubbedGet = scrubMultiMap(originalGet, fieldPatterns); + Map scrubbedPost = scrubObjectMap(originalPost, fieldPatterns, 0); + Map scrubbedMetadata = scrubObjectMap(originalMetadata, fieldPatterns, 0); + String scrubbedQueryString = scrubQueryString(originalQueryString, fieldPatterns); + + boolean changed = !equal(originalUrl, scrubbedUrl) + || scrubbedHeaders != originalHeaders + || scrubbedParams != originalParams + || scrubbedGet != originalGet + || scrubbedPost != originalPost + || scrubbedMetadata != originalMetadata + || !equal(originalQueryString, scrubbedQueryString); + + if (!changed) { + return req; + } + + return new Request.Builder(req) + .url(scrubbedUrl) + .headers(scrubbedHeaders) + .params(scrubbedParams) + .get(scrubbedGet) + .post(scrubbedPost) + .metadata(scrubbedMetadata) + .queryString(scrubbedQueryString) + .build(); + } + + private Body scrubBody(Body body) { + if (body == null || fieldPatterns.isEmpty()) { + return body; + } + + BodyContent originalContent = body.getContents(); + List originalThreads = body.getRollbarThreads(); + + BodyContent scrubbedContent = scrubBodyContent(originalContent); + List scrubbedThreads = scrubThreads(originalThreads); + + if (scrubbedContent == originalContent && scrubbedThreads == originalThreads) { + return body; + } + + return new Body.Builder(body) + .bodyContent(scrubbedContent) + .rollbarThreads(scrubbedThreads) + .build(); + } + + private BodyContent scrubBodyContent(BodyContent content) { + if (content instanceof Trace) { + return scrubTrace((Trace) content); + } else if (content instanceof TraceChain) { + return scrubTraceChain((TraceChain) content); + } + return content; + } + + /** + * The initial thread carries the same frames as the top-level body content, so its locals must + * be scrubbed too, otherwise the {@code threads} entry leaks what {@code trace} redacted. + */ + private List scrubThreads(List threads) { + if (threads == null || threads.isEmpty()) { + return threads; + } + List scrubbed = new ArrayList<>(threads.size()); + boolean anyChanged = false; + for (RollbarThread thread : threads) { + RollbarThread st = scrubThread(thread); + scrubbed.add(st); + if (st != thread) { + anyChanged = true; + } + } + return anyChanged ? scrubbed : threads; + } + + private RollbarThread scrubThread(RollbarThread thread) { + if (thread == null || thread.getGroup() == null) { + return thread; + } + TraceChain chain = thread.getGroup().getTraceChain(); + TraceChain scrubbedChain = scrubTraceChain(chain); + if (scrubbedChain == chain) { + return thread; + } + return new RollbarThread.Builder(thread).group(new Group(scrubbedChain)).build(); + } + + private TraceChain scrubTraceChain(TraceChain chain) { + if (chain == null) { + return null; + } + List traces = chain.getTraces(); + if (traces == null || traces.isEmpty()) { + return chain; + } + List scrubbed = new ArrayList<>(traces.size()); + boolean anyChanged = false; + for (Trace trace : traces) { + Trace st = scrubTrace(trace); + scrubbed.add(st); + if (st != trace) { + anyChanged = true; + } + } + if (!anyChanged) { + return chain; + } + return new TraceChain.Builder(chain).traces(scrubbed).build(); + } + + private Trace scrubTrace(Trace trace) { + if (trace == null) { + return null; + } + List frames = trace.getFrames(); + if (frames == null || frames.isEmpty()) { + return trace; + } + List scrubbed = new ArrayList<>(frames.size()); + boolean anyChanged = false; + for (Frame frame : frames) { + Frame sf = scrubFrame(frame); + scrubbed.add(sf); + if (sf != frame) { + anyChanged = true; + } + } + if (!anyChanged) { + return trace; + } + return new Trace.Builder(trace).frames(scrubbed).build(); + } + + private Frame scrubFrame(Frame frame) { + if (frame == null) { + return null; + } + Map locals = frame.getLocals(); + Map scrubbedLocals = scrubObjectMap(locals, fieldPatterns, 0); + if (scrubbedLocals == locals) { + return frame; + } + return new Frame.Builder(frame).locals(scrubbedLocals).build(); + } + + private Map scrubHeaders(Map map) { + if (map == null) { + return null; + } + Map result = null; + for (Map.Entry entry : map.entrySet()) { + String key = entry.getKey(); + if (matchesDefaultHeader(key) || matchesAny(key, fieldPatterns)) { + if (result == null) { + result = new HashMap<>(map); + } + result.put(key, SCRUBBED_VALUE); + } + } + return result != null ? result : map; + } + + /** + * Scrubs a flat string map against the configured keys only. The built-in header deny-list is + * deliberately not applied here: it names HTTP headers, and a routing parameter such as + * {@code /cookie/:id} is not one. This keeps routing params consistent with the GET/POST + * parameter maps, which also match on {@code redactedKeys} alone. + */ + private Map scrubStringMap(Map map, List patterns) { + if (map == null || patterns.isEmpty()) { + return map; + } + Map result = null; + for (Map.Entry entry : map.entrySet()) { + String key = entry.getKey(); + if (matchesAny(key, patterns)) { + if (result == null) { + result = new HashMap<>(map); + } + result.put(key, SCRUBBED_VALUE); + } + } + return result != null ? result : map; + } + + @SuppressWarnings("unchecked") + private Map scrubObjectMap(Map map, List patterns, + int depth) { + if (map == null || patterns.isEmpty()) { + return map; + } + // scrubMap only ever copies keys across, so a Map in stays one on the way out. + return (Map) scrubMap(map, patterns, depth); + } + + /** + * Recursively scrubs a nested value. Maps are scrubbed by key; collections and object arrays are + * traversed so that the maps they contain are scrubbed too, preserving the surrounding shape. + * Every container counts as one level against {@code MAX_SCRUB_DEPTH}, which also terminates + * cyclic structures. Anything else is returned untouched. + */ + private Object scrubNested(Object value, List patterns, int depth) { + if (depth >= MAX_SCRUB_DEPTH) { + return value; + } + if (value instanceof Map) { + return scrubMap((Map) value, patterns, depth + 1); + } + if (value instanceof Collection) { + return scrubCollection((Collection) value, patterns, depth + 1); + } + if (value instanceof Object[]) { + return scrubArray((Object[]) value, patterns, depth + 1); + } + return value; + } + + private Object scrubMap(Map map, List patterns, int depth) { + Map result = null; + for (Map.Entry entry : map.entrySet()) { + Object key = entry.getKey(); + Object value = entry.getValue(); + // A non-String key cannot match a redactedKeys pattern, but its value is still traversed. + boolean keyMatches = key instanceof String && matchesAny((String) key, patterns); + Object scrubbed = keyMatches ? SCRUBBED_VALUE : scrubNested(value, patterns, depth); + if (keyMatches || scrubbed != value) { + if (result == null) { + result = new LinkedHashMap<>(map); + } + result.put(key, scrubbed); + } + } + return result != null ? result : map; + } + + private Object scrubCollection(Collection collection, List patterns, int depth) { + if (collection.isEmpty()) { + return collection; + } + List scrubbed = new ArrayList<>(collection.size()); + boolean changed = false; + for (Object element : collection) { + Object scrubbedElement = scrubNested(element, patterns, depth); + scrubbed.add(scrubbedElement); + if (scrubbedElement != element) { + changed = true; + } + } + if (!changed) { + return collection; + } + // Sets keep set semantics; any other Collection serializes as a JSON array either way. + // A SortedSet is deliberately downgraded to insertion order: a rebuilt map is not Comparable. + return collection instanceof Set ? new LinkedHashSet<>(scrubbed) : scrubbed; + } + + private Object scrubArray(Object[] array, List patterns, int depth) { + Object[] result = null; + for (int i = 0; i < array.length; i++) { + Object scrubbedElement = scrubNested(array[i], patterns, depth); + if (scrubbedElement != array[i]) { + if (result == null) { + // Object[] rather than array.clone(): a rebuilt value may not fit the original component + // type (e.g. a HashMap[] receiving a LinkedHashMap), which would throw + // ArrayStoreException. + result = new Object[array.length]; + System.arraycopy(array, 0, result, 0, array.length); + } + result[i] = scrubbedElement; + } + } + return result != null ? result : array; + } + + private Map> scrubMultiMap(Map> map, + List patterns) { + if (map == null || patterns.isEmpty()) { + return map; + } + Map> result = null; + for (Map.Entry> entry : map.entrySet()) { + if (matchesAny(entry.getKey(), patterns)) { + if (result == null) { + result = new HashMap<>(map); + } + result.put(entry.getKey(), Collections.singletonList(SCRUBBED_VALUE)); + } + } + return result != null ? result : map; + } + + private String scrubQueryString(String queryString, List patterns) { + if (queryString == null || queryString.isEmpty() || patterns.isEmpty()) { + return queryString; + } + String[] pairs = queryString.split("&", -1); + boolean changed = false; + String[] output = new String[pairs.length]; + for (int i = 0; i < pairs.length; i++) { + String pair = pairs[i]; + int eq = pair.indexOf('='); + // A value-less param (e.g. "?token") is treated as key-only and scrubbed the same way. + String key = eq >= 0 ? pair.substring(0, eq) : pair; + if (matchesAny(key, patterns) || matchesAny(decodeParamName(key), patterns)) { + output[i] = key + "=" + SCRUBBED_VALUE; + changed = true; + } else { + output[i] = pair; + } + } + if (!changed) { + return queryString; + } + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < output.length; i++) { + if (i > 0) { + sb.append('&'); + } + sb.append(output[i]); + } + return sb.toString(); + } + + /** + * Percent-decodes a query parameter name. Malformed escapes (e.g. {@code %zz}) are left as-is + * rather than failing the transform: the raw form is still matched by the caller. + */ + private static String decodeParamName(String key) { + if (key.indexOf('%') < 0 && key.indexOf('+') < 0) { + return key; + } + try { + return URLDecoder.decode(key, "UTF-8"); + } catch (UnsupportedEncodingException | IllegalArgumentException e) { + return key; + } + } + + private static boolean matchesDefaultHeader(String key) { + return DEFAULT_HEADERS.contains(key.toLowerCase(Locale.ROOT)); + } + + private static boolean matchesAny(String key, List patterns) { + for (Pattern p : patterns) { + if (p.matcher(key).find()) { + return true; + } + } + return false; + } + + private static boolean equal(String a, String b) { + return a == null ? b == null : a.equals(b); + } +} diff --git a/rollbar-java/src/test/java/com/rollbar/notifier/RollbarRecordTelemetryTest.java b/rollbar-java/src/test/java/com/rollbar/notifier/RollbarRecordTelemetryTest.java index e645fae3..efc7c774 100644 --- a/rollbar-java/src/test/java/com/rollbar/notifier/RollbarRecordTelemetryTest.java +++ b/rollbar-java/src/test/java/com/rollbar/notifier/RollbarRecordTelemetryTest.java @@ -115,6 +115,54 @@ public void shouldRecordANavigationEventWithClientSourceWhenThePlatformIsAndroid verify(telemetryEventTracker).recordNavigationEventFor(level, Source.CLIENT, from, to); } + @Test + public void shouldSanitizeTheNetworkEventUrlWithTheDefaultSanitizer() { + RollbarBase sut = new RollbarBaseImpl(getConfigWith("any"), dummyFactory, null); + + sut.recordNetworkEventFor(level, "GET", "https://user:pass@example.com/p?token=secret#f", "500"); + + verify(telemetryEventTracker) + .recordNetworkEventFor(level, Source.SERVER, "GET", "https://example.com/p", "500"); + } + + @Test + public void shouldSanitizeTheNetworkEventUrlWithTheConfiguredSanitizer() { + Config config = withAccessToken("dummy token") + .telemetryEventTracker(telemetryEventTracker) + .urlSanitizer(url -> "sanitized") + .build(); + RollbarBase sut = new RollbarBaseImpl(config, dummyFactory, null); + + sut.recordNetworkEventFor(level, "GET", "https://example.com/p?token=secret", "500"); + + verify(telemetryEventTracker) + .recordNetworkEventFor(level, Source.SERVER, "GET", "sanitized", "500"); + } + + @Test + public void shouldUseTheReconfiguredSanitizerForLaterNetworkEvents() { + RollbarBaseImpl sut = new RollbarBaseImpl(getConfigWith("any"), dummyFactory, null); + + sut.reconfigure(withAccessToken("dummy token") + .telemetryEventTracker(telemetryEventTracker) + .urlSanitizer(url -> "reconfigured") + .build()); + sut.recordNetworkEventFor(level, "GET", "https://example.com/p?token=secret", "500"); + + verify(telemetryEventTracker) + .recordNetworkEventFor(level, Source.SERVER, "GET", "reconfigured", "500"); + } + + @Test + public void shouldRecordANetworkEventWithANullUrl() { + RollbarBase sut = new RollbarBaseImpl(getConfigWith("any"), dummyFactory, null); + + sut.recordNetworkEventFor(level, "GET", null, "500"); + + verify(telemetryEventTracker) + .recordNetworkEventFor(level, Source.SERVER, "GET", null, "500"); + } + private Config getConfigWith(String platform) { return withAccessToken("dummy token") .telemetryEventTracker(telemetryEventTracker) @@ -128,6 +176,10 @@ protected RollbarBaseImpl(Config config, BodyFactory bodyFactory, Void emptyResu super(config, bodyFactory, emptyResult); } + void reconfigure(Config config) { + configure(config); + } + @Override protected Void sendPayload(Config config, Payload payload) { return null; diff --git a/rollbar-java/src/test/java/com/rollbar/notifier/scrubbing/ScrubDataTransformerTest.java b/rollbar-java/src/test/java/com/rollbar/notifier/scrubbing/ScrubDataTransformerTest.java new file mode 100644 index 00000000..d72817ad --- /dev/null +++ b/rollbar-java/src/test/java/com/rollbar/notifier/scrubbing/ScrubDataTransformerTest.java @@ -0,0 +1,766 @@ +package com.rollbar.notifier.scrubbing; + +import com.rollbar.api.payload.data.Data; +import com.rollbar.api.payload.data.Request; +import com.rollbar.api.payload.data.body.Body; +import com.rollbar.api.payload.data.body.Frame; +import com.rollbar.api.payload.data.body.Group; +import com.rollbar.api.payload.data.body.RollbarThread; +import com.rollbar.api.payload.data.body.Trace; +import com.rollbar.api.payload.data.body.TraceChain; +import com.rollbar.api.scrubbing.StringUrlSanitizer; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.*; + +public class ScrubDataTransformerTest { + + private static final StringUrlSanitizer NO_OP_SANITIZER = url -> url; + + // --- helpers --- + + private static Data dataWithRequest(Request request) { + return new Data.Builder() + .environment("test") + .request(request) + .build(); + } + + private static Data dataWithCustom(Map custom) { + return new Data.Builder() + .environment("test") + .custom(custom) + .build(); + } + + private static Map headers(String... kvPairs) { + Map map = new HashMap<>(); + for (int i = 0; i < kvPairs.length; i += 2) { + map.put(kvPairs[i], kvPairs[i + 1]); + } + return map; + } + + private static Map> getParams(String key, String value) { + Map> map = new HashMap<>(); + map.put(key, Collections.singletonList(value)); + return map; + } + + private static Map objectMap(String... kvPairs) { + Map map = new HashMap<>(); + for (int i = 0; i < kvPairs.length; i += 2) { + map.put(kvPairs[i], kvPairs[i + 1]); + } + return map; + } + + // --- default header deny-list --- + + @Test + public void authorizationHeaderRedacted() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder() + .headers(headers("Authorization", "Bearer secret-token", "Content-Type", "application/json")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("Authorization")); + assertEquals("application/json", result.getRequest().getHeaders().get("Content-Type")); + } + + @Test + public void cookieHeaderRedacted() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder() + .headers(headers("Cookie", "session=abc123")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("Cookie")); + } + + @Test + public void setCookieHeaderRedacted() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder() + .headers(headers("Set-Cookie", "session=abc123; HttpOnly")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("Set-Cookie")); + } + + @Test + public void xApiKeyHeaderRedacted() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder() + .headers(headers("X-Api-Key", "key-12345")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("X-Api-Key")); + } + + @Test + public void caseInsensitiveHeaderMatching() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder() + .headers(headers("AUTHORIZATION", "Basic xyz", "authorization", "Bearer abc")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("AUTHORIZATION")); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("authorization")); + } + + // --- user redactedKeys --- + + @Test + public void userKeyRedactsHeaders() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("X-My-Secret"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .headers(headers("X-My-Secret", "sensitive", "Content-Type", "text/plain")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("X-My-Secret")); + assertEquals("text/plain", result.getRequest().getHeaders().get("Content-Type")); + } + + @Test + public void userKeyRedactsGetParams() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("apiToken"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .get(getParams("apiToken", "secret-value")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, + result.getRequest().getGet().get("apiToken").get(0)); + } + + @Test + public void userKeyRedactsPostParams() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map post = objectMap("password", "hunter2", "username", "alice"); + Request req = new Request.Builder().post(post).build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getPost().get("password")); + assertEquals("alice", result.getRequest().getPost().get("username")); + } + + @Test + public void userKeyRedactsCustomMap() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("mySecret"), NO_OP_SANITIZER); + Map custom = objectMap("mySecret", "hidden", "other", "visible"); + Data result = t.transform(dataWithCustom(custom)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getCustom().get("mySecret")); + assertEquals("visible", result.getCustom().get("other")); + } + + @Test + public void userKeyRedactsRoutingParams() { + // e.g. a /reset/:token route populating Request.params. + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .params(headers("token", "reset-token-abc", "userId", "42")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getParams().get("token")); + assertEquals("42", result.getRequest().getParams().get("userId")); + } + + @Test + public void routingParamsNotMatchedByHeaderDenyList() { + // The built-in deny-list names HTTP headers; a routing param called "cookie" is not one. + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder() + .params(headers("cookie", "chocolate-chip")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals("chocolate-chip", result.getRequest().getParams().get("cookie")); + } + + @Test + public void userKeyRedactsMetadata() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("apiKey"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .metadata(objectMap("apiKey", "key-12345", "region", "us-east-1")) + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getMetadata().get("apiKey")); + assertEquals("us-east-1", result.getRequest().getMetadata().get("region")); + } + + @Test + public void nestedMetadataKeysScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map inner = objectMap("password", "hunter2", "user", "alice"); + Map metadata = new HashMap<>(); + metadata.put("auth", inner); + Request req = new Request.Builder().metadata(metadata).build(); + Data result = t.transform(dataWithRequest(req)); + @SuppressWarnings("unchecked") + Map scrubbed = (Map) result.getRequest().getMetadata().get("auth"); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, scrubbed.get("password")); + assertEquals("alice", scrubbed.get("user")); + } + + @Test + public void nullParamsAndMetadataNoNpe() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Request req = new Request.Builder().url("https://example.com").build(); + Data result = t.transform(dataWithRequest(req)); + assertNull(result.getRequest().getParams()); + assertNull(result.getRequest().getMetadata()); + } + + @Test + public void userKeyRegexMatchesMultipleKeys() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList(".*[Pp]assword.*"), NO_OP_SANITIZER); + Map custom = objectMap( + "passwordHash", "xxx", + "oldPassword", "yyy", + "username", "alice" + ); + Data result = t.transform(dataWithCustom(custom)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getCustom().get("passwordHash")); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getCustom().get("oldPassword")); + assertEquals("alice", result.getCustom().get("username")); + } + + // --- URL sanitizer --- + + @Test + public void urlSanitizedViaProvidedSanitizer() { + StringUrlSanitizer strip = url -> "https://example.com/clean"; + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), strip); + Request req = new Request.Builder() + .url("https://example.com/api?secret=xyz") + .build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals("https://example.com/clean", result.getRequest().getUrl()); + } + + // --- queryString --- + + @Test + public void queryStringValueRedacted() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .queryString("token=secret&page=1") + .build(); + Data result = t.transform(dataWithRequest(req)); + String qs = result.getRequest().getQueryString(); + assertTrue(qs.contains("token=" + ScrubDataTransformer.SCRUBBED_VALUE)); + assertTrue(qs.contains("page=1")); + } + + @Test + public void queryStringUnchangedWhenNoMatch() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + String qs = "page=1&sort=asc"; + Request req = new Request.Builder().queryString(qs).build(); + Data result = t.transform(dataWithRequest(req)); + assertSame(qs, result.getRequest().getQueryString()); + } + + @Test + public void percentEncodedQueryParamNameRedacted() { + // getQueryString() is raw, so "pass%77ord" is semantically the "password" param. + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .queryString("pass%77ord=hunter2&page=1") + .build(); + Data result = t.transform(dataWithRequest(req)); + String qs = result.getRequest().getQueryString(); + assertFalse(qs.contains("hunter2")); + // The original encoding of the key is preserved; only the value is replaced. + assertTrue(qs.contains("pass%77ord=" + ScrubDataTransformer.SCRUBBED_VALUE)); + assertTrue(qs.contains("page=1")); + } + + @Test + public void fullyPercentEncodedQueryParamNameRedacted() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .queryString("%70%61%73%73%77%6F%72%64=hunter2") + .build(); + Data result = t.transform(dataWithRequest(req)); + assertFalse(result.getRequest().getQueryString().contains("hunter2")); + } + + @Test + public void plusEncodedQueryParamNameRedacted() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("user password"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .queryString("user+password=hunter2") + .build(); + Data result = t.transform(dataWithRequest(req)); + assertFalse(result.getRequest().getQueryString().contains("hunter2")); + } + + @Test + public void malformedEscapeInQueryParamNameFallsBackToRawMatch() { + // %zz is not a valid escape; decoding fails and the raw name is matched instead. + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Request req = new Request.Builder() + .queryString("token%zz=secret&page=1") + .build(); + Data result = t.transform(dataWithRequest(req)); + String qs = result.getRequest().getQueryString(); + assertFalse(qs.contains("secret")); + assertTrue(qs.contains("page=1")); + } + + @Test + public void malformedEscapeInNonMatchingQueryParamPassedThrough() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + String qs = "page%zz=1"; + Request req = new Request.Builder().queryString(qs).build(); + Data result = t.transform(dataWithRequest(req)); + assertSame(qs, result.getRequest().getQueryString()); + } + + @Test + public void encodedValueLessQueryParamScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Request req = new Request.Builder().queryString("t%6Fken").build(); + Data result = t.transform(dataWithRequest(req)); + assertEquals("t%6Fken=" + ScrubDataTransformer.SCRUBBED_VALUE, + result.getRequest().getQueryString()); + } + + // --- Frame.locals --- + + @Test + public void frameLocalsMatchingKeysScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map locals = objectMap("password", "secret", "userId", "42"); + Frame frame = new Frame.Builder().locals(locals).build(); + Trace trace = new Trace.Builder().frames(Collections.singletonList(frame)).build(); + Body body = new Body.Builder().bodyContent(trace).build(); + Data data = new Data.Builder().environment("test").body(body).build(); + + Data result = t.transform(data); + + List frames = ((Trace) result.getBody().getContents()).getFrames(); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, frames.get(0).getLocals().get("password")); + assertEquals("42", frames.get(0).getLocals().get("userId")); + } + + // --- null-safety --- + + @Test + public void nullRequestReturnsDataUnchanged() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Data data = new Data.Builder().environment("test").build(); + assertSame(data, t.transform(data)); + } + + @Test + public void nullHeadersMapNoNpe() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder().url("https://example.com").build(); // headers null + Data result = t.transform(dataWithRequest(req)); + assertNotNull(result); + } + + @Test + public void nullCustomMapNoNpe() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("secret"), NO_OP_SANITIZER); + Data data = new Data.Builder().environment("test").build(); // custom null + Data result = t.transform(data); + assertNotNull(result); + } + + @Test + public void noMatchReturnsSameDataInstance() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Request req = new Request.Builder() + .url("https://example.com") + .headers(headers("Content-Type", "application/json")) + .build(); + Data data = dataWithRequest(req); + assertSame(data, t.transform(data)); + } + + @Test + public void emptyRedactedKeysOnlyScrubsDefaultHeaders() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + Map custom = objectMap("myApiKey", "visible"); + Map hdrs = headers("Authorization", "Bearer xyz", "Content-Type", "text/html"); + Request req = new Request.Builder().headers(hdrs).build(); + Data data = new Data.Builder().environment("test").request(req).custom(custom).build(); + + Data result = t.transform(data); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getRequest().getHeaders().get("Authorization")); + assertEquals("text/html", result.getRequest().getHeaders().get("Content-Type")); + // custom key not in default deny-list → not scrubbed + assertEquals("visible", result.getCustom().get("myApiKey")); + } + + @Test + public void nullDataReturnsNull() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.emptyList(), NO_OP_SANITIZER); + assertNull(t.transform(null)); + } + + // --- value-less query params (B1 fix) --- + + @Test + public void valueLessQueryParamScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Request req = new Request.Builder().queryString("token&page=1").build(); + Data result = t.transform(dataWithRequest(req)); + String qs = result.getRequest().getQueryString(); + assertTrue(qs.contains("token=" + ScrubDataTransformer.SCRUBBED_VALUE)); + assertTrue(qs.contains("page=1")); + } + + @Test + public void valueLessQueryParamNoMatchPassedThrough() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Request req = new Request.Builder().queryString("debug&page=1").build(); + Data result = t.transform(dataWithRequest(req)); + assertSame(req.getQueryString(), result.getRequest().getQueryString()); + } + + // --- nested map scrubbing (B4 fix) --- + + @Test + public void nestedCustomMapKeysScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map inner = objectMap("password", "hunter2", "user", "alice"); + Map custom = new HashMap<>(); + custom.put("auth", inner); + custom.put("visible", "yes"); + Data result = t.transform(dataWithCustom(custom)); + @SuppressWarnings("unchecked") + Map scrubbed = (Map) result.getCustom().get("auth"); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, scrubbed.get("password")); + assertEquals("alice", scrubbed.get("user")); + assertEquals("yes", result.getCustom().get("visible")); + } + + @Test + public void nestedFrameLocalsKeysScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Map inner = objectMap("token", "secret-token", "count", "5"); + Map locals = new HashMap<>(); + locals.put("credentials", inner); + locals.put("userId", "42"); + Frame frame = new Frame.Builder().locals(locals).build(); + Trace trace = new Trace.Builder().frames(Collections.singletonList(frame)).build(); + Body body = new Body.Builder().bodyContent(trace).build(); + Data data = new Data.Builder().environment("test").body(body).build(); + + Data result = t.transform(data); + List frames = ((Trace) result.getBody().getContents()).getFrames(); + @SuppressWarnings("unchecked") + Map scrubbedInner = + (Map) frames.get(0).getLocals().get("credentials"); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, scrubbedInner.get("token")); + assertEquals("5", scrubbedInner.get("count")); + assertEquals("42", frames.get(0).getLocals().get("userId")); + } + + @Test + public void nestedMapParentKeyMatchScrubsEntireValue() { + // When the top-level key itself matches, the whole nested map is replaced, not recursed. + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("credentials"), NO_OP_SANITIZER); + Map inner = objectMap("password", "hunter2"); + Map custom = new HashMap<>(); + custom.put("credentials", inner); + Data result = t.transform(dataWithCustom(custom)); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, result.getCustom().get("credentials")); + } + + @Test + public void threadFrameLocalsScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Body body = new Body.Builder() + .bodyContent(traceWithLocals(objectMap("password", "hunter2", "userId", "42"))) + .rollbarThreads(Collections.singletonList( + threadWithLocals(objectMap("password", "hunter2", "userId", "42")))) + .build(); + Data data = new Data.Builder().environment("test").body(body).build(); + + Data result = t.transform(data); + + Map locals = threadLocals(result.getBody(), 0); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, locals.get("password")); + assertEquals("42", locals.get("userId")); + // The top-level trace is still scrubbed. + List frames = ((Trace) result.getBody().getContents()).getFrames(); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, frames.get(0).getLocals().get("password")); + } + + @Test + public void threadFrameLocalsScrubbedWhenBodyContentHasNoMatch() { + // Regression: the threads entry must be scrubbed even when the body content needs no change. + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Body body = new Body.Builder() + .bodyContent(traceWithLocals(objectMap("userId", "42"))) + .rollbarThreads(Collections.singletonList( + threadWithLocals(objectMap("token", "secret-token")))) + .build(); + Data data = new Data.Builder().environment("test").body(body).build(); + + Data result = t.transform(data); + + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, + threadLocals(result.getBody(), 0).get("token")); + } + + @Test + public void threadsWithNoMatchReturnSameBodyInstance() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Body body = new Body.Builder() + .bodyContent(traceWithLocals(objectMap("userId", "42"))) + .rollbarThreads(Collections.singletonList(threadWithLocals(objectMap("userId", "42")))) + .build(); + Data data = new Data.Builder().environment("test").body(body).build(); + + assertSame(data, t.transform(data)); + } + + @Test + public void nullThreadsNoNpe() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Body body = new Body.Builder() + .bodyContent(traceWithLocals(objectMap("password", "hunter2"))) + .build(); // rollbarThreads null + Data data = new Data.Builder().environment("test").body(body).build(); + + Data result = t.transform(data); + + List frames = ((Trace) result.getBody().getContents()).getFrames(); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, frames.get(0).getLocals().get("password")); + assertNull(result.getBody().getRollbarThreads()); + } + + // --- collections and arrays (P1 fix) --- + + @Test + public void listOfMapsInCustomScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map custom = new HashMap<>(); + custom.put("users", Collections.singletonList(objectMap("password", "hunter2", "name", "alice"))); + Data result = t.transform(dataWithCustom(custom)); + + List users = (List) result.getCustom().get("users"); + Map user = (Map) users.get(0); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, user.get("password")); + assertEquals("alice", user.get("name")); + } + + @Test + public void arrayOfMapsInCustomScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map custom = new HashMap<>(); + custom.put("users", new Object[] {objectMap("password", "hunter2", "name", "alice")}); + Data result = t.transform(dataWithCustom(custom)); + + Object[] users = (Object[]) result.getCustom().get("users"); + Map user = (Map) users[0]; + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, user.get("password")); + assertEquals("alice", user.get("name")); + } + + @Test + public void nestedListScrubbedInRequestPost() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map post = new HashMap<>(); + post.put("users", Collections.singletonList(objectMap("password", "hunter2"))); + Request req = new Request.Builder().post(post).build(); + + Data result = t.transform(dataWithRequest(req)); + + List users = (List) result.getRequest().getPost().get("users"); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, ((Map) users.get(0)).get("password")); + } + + @Test + public void nestedListScrubbedInRequestMetadata() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map metadata = new HashMap<>(); + metadata.put("users", Collections.singletonList(objectMap("password", "hunter2"))); + Request req = new Request.Builder().metadata(metadata).build(); + + Data result = t.transform(dataWithRequest(req)); + + List users = (List) result.getRequest().getMetadata().get("users"); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, ((Map) users.get(0)).get("password")); + } + + @Test + public void nestedArrayScrubbedInFrameLocals() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("token"), NO_OP_SANITIZER); + Map locals = new HashMap<>(); + locals.put("sessions", new Object[] {objectMap("token", "secret-token")}); + Body body = new Body.Builder().bodyContent(traceWithLocals(locals)).build(); + Data data = new Data.Builder().environment("test").body(body).build(); + + Data result = t.transform(data); + + List frames = ((Trace) result.getBody().getContents()).getFrames(); + Object[] sessions = (Object[]) frames.get(0).getLocals().get("sessions"); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, ((Map) sessions[0]).get("token")); + } + + @Test + public void listOrderAndSizePreservedWhenScrubbing() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map custom = new HashMap<>(); + custom.put("rows", Arrays.asList(objectMap("password", "hunter2"), "plain", objectMap("name", "bob"))); + Data result = t.transform(dataWithCustom(custom)); + + Object scrubbed = result.getCustom().get("rows"); + assertTrue(scrubbed instanceof List); + List rows = (List) scrubbed; + assertEquals(3, rows.size()); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, ((Map) rows.get(0)).get("password")); + assertEquals("plain", rows.get(1)); + assertEquals("bob", ((Map) rows.get(2)).get("name")); + } + + @Test + public void setShapePreservedWhenScrubbing() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Set rows = new LinkedHashSet<>(); + rows.add(objectMap("password", "hunter2")); + rows.add("plain"); + Map custom = new HashMap<>(); + custom.put("rows", rows); + Data result = t.transform(dataWithCustom(custom)); + + Object scrubbed = result.getCustom().get("rows"); + assertTrue(scrubbed instanceof Set); + Set scrubbedRows = (Set) scrubbed; + assertEquals(2, scrubbedRows.size()); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, + ((Map) scrubbedRows.iterator().next()).get("password")); + } + + @Test + public void typedArrayScrubbedWithoutArrayStoreException() { + // The rebuilt map may not fit the original component type, so the array is widened on copy. + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + HashMap[] rows = new HashMap[] {(HashMap) objectMap("password", "hunter2")}; + Map custom = new HashMap<>(); + custom.put("rows", rows); + + Data result = t.transform(dataWithCustom(custom)); + + Object[] scrubbed = (Object[]) result.getCustom().get("rows"); + assertEquals(1, scrubbed.length); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, ((Map) scrubbed[0]).get("password")); + } + + @Test + public void collectionWithNoMatchReturnsSameInstances() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + List users = Collections.singletonList(objectMap("name", "alice")); + Map custom = new HashMap<>(); + custom.put("users", users); + Data data = dataWithCustom(custom); + + Data result = t.transform(data); + + assertSame(data, result); + assertSame(users, result.getCustom().get("users")); + } + + @Test + public void collectionNestingWithinDepthCapScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map custom = new HashMap<>(); + custom.put("root", nestInLists(objectMap("password", "hunter2"), 7)); + + Data result = t.transform(dataWithCustom(custom)); + + Map leaf = (Map) unwrapLists(result.getCustom().get("root"), 7); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, leaf.get("password")); + } + + @Test + public void collectionNestingBeyondDepthCapNotScrubbed() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map custom = new HashMap<>(); + custom.put("root", nestInLists(objectMap("password", "hunter2"), 8)); + + Data result = t.transform(dataWithCustom(custom)); + + Map leaf = (Map) unwrapLists(result.getCustom().get("root"), 8); + assertEquals("hunter2", leaf.get("password")); + } + + @Test + public void selfReferencingCollectionTerminates() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + List cycle = new ArrayList<>(); + cycle.add(objectMap("password", "hunter2")); + cycle.add(cycle); + Map custom = new HashMap<>(); + custom.put("cycle", cycle); + + Data result = t.transform(dataWithCustom(custom)); + + List scrubbed = (List) result.getCustom().get("cycle"); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, + ((Map) scrubbed.get(0)).get("password")); + } + + @Test + public void nonStringMapKeysInsideCollectionDoNotThrow() { + ScrubDataTransformer t = new ScrubDataTransformer(Collections.singletonList("password"), NO_OP_SANITIZER); + Map byId = new HashMap<>(); + byId.put(1, objectMap("password", "hunter2")); + Map custom = new HashMap<>(); + custom.put("rows", Collections.singletonList(byId)); + + Data result = t.transform(dataWithCustom(custom)); + + Map scrubbedById = (Map) ((List) result.getCustom().get("rows")).get(0); + assertEquals(ScrubDataTransformer.SCRUBBED_VALUE, + ((Map) scrubbedById.get(1)).get("password")); + } + + private static Object nestInLists(Object leaf, int levels) { + Object current = leaf; + for (int i = 0; i < levels; i++) { + current = new ArrayList<>(Collections.singletonList(current)); + } + return current; + } + + private static Object unwrapLists(Object value, int levels) { + Object current = value; + for (int i = 0; i < levels; i++) { + current = ((List) current).get(0); + } + return current; + } + + private static Trace traceWithLocals(Map locals) { + Frame frame = new Frame.Builder().locals(locals).build(); + return new Trace.Builder().frames(Collections.singletonList(frame)).build(); + } + + private static RollbarThread threadWithLocals(Map locals) { + TraceChain chain = new TraceChain.Builder() + .traces(Collections.singletonList(traceWithLocals(locals))) + .build(); + return new RollbarThread("main", "1", "5", "RUNNABLE", new Group(chain)); + } + + private static Map threadLocals(Body body, int threadIndex) { + return body.getRollbarThreads().get(threadIndex) + .getGroup().getTraceChain().getTraces().get(0) + .getFrames().get(0).getLocals(); + } +} diff --git a/rollbar-okhttp/src/main/java/com/rollbar/okhttp/RollbarOkHttpInterceptor.java b/rollbar-okhttp/src/main/java/com/rollbar/okhttp/RollbarOkHttpInterceptor.java index edbd709e..ab865842 100644 --- a/rollbar-okhttp/src/main/java/com/rollbar/okhttp/RollbarOkHttpInterceptor.java +++ b/rollbar-okhttp/src/main/java/com/rollbar/okhttp/RollbarOkHttpInterceptor.java @@ -1,6 +1,8 @@ package com.rollbar.okhttp; import com.rollbar.api.payload.data.Level; +import com.rollbar.api.scrubbing.DefaultUrlSanitizer; +import com.rollbar.api.scrubbing.StringUrlSanitizer; import java.io.IOException; import java.util.Objects; @@ -18,18 +20,29 @@ public class RollbarOkHttpInterceptor implements Interceptor { private static final Logger LOGGER = LoggerFactory.getLogger(RollbarOkHttpInterceptor.class); private static final UrlSanitizer DEFAULT_URL_SANITIZER = - url -> url - .newBuilder() - .username("") - .password("") - .query(null) - .fragment(null) - .build() - .toString(); + url -> DefaultUrlSanitizer.INSTANCE.sanitize(url.toString()); private final NetworkTelemetryRecorder recorder; private final UrlSanitizer urlSanitizer; + /** + * Creates an interceptor that sanitizes URLs with the same {@link StringUrlSanitizer} used by + * the notifier configuration, so both paths redact identically. + * + *

This is a static factory rather than a constructor overload because {@link UrlSanitizer} + * and {@link StringUrlSanitizer} are both functional interfaces: overloaded constructors would + * make a lambda argument ambiguous and break existing callers. + * + * @param recorder the telemetry recorder. + * @param sanitizer the sanitizer shared with the notifier config. + * @return the interceptor. + */ + public static RollbarOkHttpInterceptor withSharedUrlSanitizer(NetworkTelemetryRecorder recorder, + StringUrlSanitizer sanitizer) { + Objects.requireNonNull(sanitizer, "sanitizer must not be null"); + return new RollbarOkHttpInterceptor(recorder, url -> sanitizer.sanitize(url.toString())); + } + public RollbarOkHttpInterceptor(NetworkTelemetryRecorder recorder) { this(recorder, DEFAULT_URL_SANITIZER); } diff --git a/rollbar-okhttp/src/test/java/com/rollbar/okhttp/RollbarOkHttpInterceptorTest.java b/rollbar-okhttp/src/test/java/com/rollbar/okhttp/RollbarOkHttpInterceptorTest.java index 2ca8f8d6..43be84cd 100644 --- a/rollbar-okhttp/src/test/java/com/rollbar/okhttp/RollbarOkHttpInterceptorTest.java +++ b/rollbar-okhttp/src/test/java/com/rollbar/okhttp/RollbarOkHttpInterceptorTest.java @@ -1,6 +1,8 @@ package com.rollbar.okhttp; import com.rollbar.api.payload.data.Level; +import com.rollbar.api.scrubbing.DefaultUrlSanitizer; +import com.rollbar.api.scrubbing.StringUrlSanitizer; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -236,6 +238,60 @@ public void customSanitizerThrows_responseStillReturnedAndRecorderNotCalled() th verify(recorder, never()).recordNetworkEvent(any(), any(), any(), any()); } + /** + * {@code DefaultUrlSanitizer.INSTANCE} is what {@code CommonConfig.urlSanitizer()} returns by + * default, so passing it here is the shared-sanitizer path a notifier user would configure. + * rollbar-okhttp only depends on rollbar-api, so the notifier config is not referenced directly. + */ + @Test + public void sharedUrlSanitizer_redactsIdenticallyToTheNotifier() throws IOException { + server.enqueue(new MockResponse().setResponseCode(500)); + + OkHttpClient sharedClient = new OkHttpClient.Builder() + .addInterceptor(RollbarOkHttpInterceptor.withSharedUrlSanitizer( + recorder, DefaultUrlSanitizer.INSTANCE)) + .build(); + + HttpUrl url = server.url("/charge") + .newBuilder() + .username("anyUser") + .password("anyPassword") + .addQueryParameter("token", "abc") + .fragment("section") + .build(); + + Response response = sharedClient.newCall(new Request.Builder().url(url).build()).execute(); + response.close(); + + verify(recorder).recordNetworkEvent( + eq(Level.CRITICAL), eq("GET"), + eq(DefaultUrlSanitizer.INSTANCE.sanitize(url.toString())), + eq("500")); + } + + @Test + public void sharedUrlSanitizer_appliesACustomSanitizer() throws IOException { + server.enqueue(new MockResponse().setResponseCode(500)); + + StringUrlSanitizer sanitizer = url -> "shared-sanitized"; + OkHttpClient sharedClient = new OkHttpClient.Builder() + .addInterceptor(RollbarOkHttpInterceptor.withSharedUrlSanitizer(recorder, sanitizer)) + .build(); + + Request request = new Request.Builder().url(server.url("/path?secret=abc")).build(); + Response response = sharedClient.newCall(request).execute(); + response.close(); + + verify(recorder).recordNetworkEvent( + eq(Level.CRITICAL), eq("GET"), eq("shared-sanitized"), eq("500")); + } + + @Test + public void sharedUrlSanitizer_rejectsANullSanitizer() { + assertThrows(NullPointerException.class, + () -> RollbarOkHttpInterceptor.withSharedUrlSanitizer(recorder, null)); + } + @Test public void customSanitizer_isAppliedToUrl() throws IOException { server.enqueue(new MockResponse().setResponseCode(500)); diff --git a/rollbar-reactive-streams/src/integTest/java/com/rollbar/reactivestreams/notifier/ScrubbingReactiveITest.java b/rollbar-reactive-streams/src/integTest/java/com/rollbar/reactivestreams/notifier/ScrubbingReactiveITest.java new file mode 100644 index 00000000..a0defef0 --- /dev/null +++ b/rollbar-reactive-streams/src/integTest/java/com/rollbar/reactivestreams/notifier/ScrubbingReactiveITest.java @@ -0,0 +1,185 @@ +package com.rollbar.reactivestreams.notifier; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static com.rollbar.reactivestreams.notifier.config.ConfigBuilder.withAccessToken; +import static java.lang.String.format; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import com.google.gson.Gson; +import com.rollbar.api.payload.data.Data; +import com.rollbar.api.payload.data.Level; +import com.rollbar.notifier.scrubbing.ScrubDataTransformer; +import com.rollbar.notifier.transformer.Transformer; +import com.rollbar.reactivestreams.notifier.config.Config; +import com.rollbar.reactivestreams.notifier.config.ConfigBuilder; +import com.rollbar.reactivestreams.notifier.sender.AsyncSender; +import com.rollbar.reactivestreams.notifier.sender.http.ApacheAsyncHttpClient; +import com.rollbar.notifier.sender.result.Response; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.reactivestreams.Publisher; +import org.reactivestreams.Subscriber; +import org.reactivestreams.Subscription; + +/** + * The reactive {@link ConfigBuilder} duplicates the {@code redactedKeys} and {@code urlSanitizer} + * plumbing of the synchronous one, so it needs its own end-to-end coverage. The scrubbing itself + * is inherited from {@code RollbarBase} and is exercised in depth by + * {@code com.rollbar.notifier.ScrubbingITest}. + */ +public class ScrubbingReactiveITest { + + private static final String ACCESS_TOKEN = UUID.randomUUID().toString(); + + private static final String SCRUBBED = ScrubDataTransformer.SCRUBBED_VALUE; + + @Rule + public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().dynamicPort()); + + private ConfigBuilder configBuilder; + + @Before + public void setUp() { + AsyncSender sender = new AsyncSender.Builder(new ApacheAsyncHttpClient.Builder().build(), + getUrl()) + .accessToken(ACCESS_TOKEN) + .build(); + + this.configBuilder = withAccessToken(ACCESS_TOKEN).sender(sender); + + stubFor(post(urlEqualTo("/api/1/item/")) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"err\":0,\"result\":{\"uuid\":\"" + UUID.randomUUID() + "\"}}"))); + } + + @Test + public void redactedKeysFromTheReactiveBuilderAreAppliedAfterTheUserTransformer() + throws Exception { + Transformer injectSecret = data -> new Data.Builder(data) + .custom(nestedCustom()) + .build(); + + Config config = configBuilder + .transformer(injectSecret) + .redactedKeys(Collections.singletonList("password")) + .build(); + + try (Rollbar rollbar = new Rollbar(config)) { + await(rollbar.error("boom")); + } + + Map custom = getValue(sentData(), "custom"); + assertThat(custom.get("password"), is(SCRUBBED)); + + List> users = getValue(custom, "users"); + assertThat(users, hasSize(1)); + assertThat(users.get(0).get("password"), is(SCRUBBED)); + } + + @Test + public void networkTelemetryUrlsAreSanitized() throws Exception { + try (Rollbar rollbar = new Rollbar(configBuilder.build())) { + rollbar.recordNetworkEventFor(Level.CRITICAL, "GET", + "https://user:pass@example.com/orders?token=secret", "500"); + await(rollbar.error("boom")); + } + + List> telemetry = getValue(sentData(), "body", "telemetry"); + assertThat(telemetry, hasSize(1)); + Map body = getValue(telemetry.get(0), "body"); + assertThat(body.get("url"), is("https://example.com/orders")); + } + + // --- helpers --- + + private static Map nestedCustom() { + Map custom = new HashMap<>(); + custom.put("password", "hunter2"); + Map user = new HashMap<>(); + user.put("password", "hunter2"); + custom.put("users", List.of(user)); + return custom; + } + + private static void await(Publisher publisher) throws InterruptedException { + CountDownLatch latch = new CountDownLatch(1); + publisher.subscribe(new Subscriber<>() { + @Override + public void onSubscribe(Subscription subscription) { + subscription.request(Long.MAX_VALUE); + } + + @Override + public void onNext(Response response) { + } + + @Override + public void onError(Throwable throwable) { + latch.countDown(); + } + + @Override + public void onComplete() { + latch.countDown(); + } + }); + assertTrue("Timed out waiting for the payload to be sent", latch.await(20, TimeUnit.SECONDS)); + } + + /** The parsed {@code data} object of the nth payload WireMock received. */ + @SuppressWarnings("unchecked") + private Map sentData() { + List requests = + WireMock.findAll(postRequestedFor(urlEqualTo("/api/1/item/"))); + Map payload = + new Gson().fromJson(requests.get(0).getBodyAsString(), Map.class); + return getValue(payload, "data"); + } + + @SuppressWarnings("unchecked") + private static T getValue(Map source, String attribute, + String... attributes) { + Object value = source.get(attribute); + + if (attributes.length == 0) { + return (T) value; + } + + if (value == null) { + throw new NullPointerException("No value with key " + attribute); + } + + Map asMap = (Map) value; + String[] newAttributes = new String[attributes.length - 1]; + System.arraycopy(attributes, 1, newAttributes, 0, newAttributes.length); + + return getValue(asMap, attributes[0], newAttributes); + } + + private String getUrl() { + return format(Locale.US, "http://localhost:%d/api/1/item/", wireMockRule.port()); + } +} diff --git a/rollbar-reactive-streams/src/main/java/com/rollbar/reactivestreams/notifier/config/ConfigBuilder.java b/rollbar-reactive-streams/src/main/java/com/rollbar/reactivestreams/notifier/config/ConfigBuilder.java index 5151e234..b4619bb4 100644 --- a/rollbar-reactive-streams/src/main/java/com/rollbar/reactivestreams/notifier/config/ConfigBuilder.java +++ b/rollbar-reactive-streams/src/main/java/com/rollbar/reactivestreams/notifier/config/ConfigBuilder.java @@ -6,6 +6,8 @@ import com.rollbar.api.payload.data.Person; import com.rollbar.api.payload.data.Request; import com.rollbar.api.payload.data.Server; +import com.rollbar.api.scrubbing.DefaultUrlSanitizer; +import com.rollbar.api.scrubbing.StringUrlSanitizer; import com.rollbar.notifier.Rollbar; import com.rollbar.notifier.config.DefaultLevels; import com.rollbar.notifier.filter.Filter; @@ -62,6 +64,8 @@ public final class ConfigBuilder { private DefaultLevels defaultLevels; private boolean truncateLargePayloads; private boolean compressPayload; + private List redactedKeys; + private StringUrlSanitizer urlSanitizer; private int maximumTelemetryData = RollbarTelemetryEventTracker.MAXIMUM_CAPACITY_FOR_TELEMETRY_EVENTS; private TelemetryEventTracker telemetryEventTracker; @@ -111,6 +115,8 @@ private ConfigBuilder(Config config) { this.compressPayload = config.compressPayload(); this.maximumTelemetryData = config.maximumTelemetryData(); this.telemetryEventTracker = config.telemetryEventTracker(); + this.redactedKeys = config.redactedKeys(); + this.urlSanitizer = config.urlSanitizer(); } private ConfigBuilder(Sender sender) { @@ -511,6 +517,31 @@ public ConfigBuilder telemetryEventTracker(TelemetryEventTracker telemetryEventT return this; } + /** + * Keys (matched as case-insensitive regex) whose values will be redacted in request headers, + * query/POST parameters, custom data, and {@code Frame.locals}. These are additive to the + * built-in header deny-list (Authorization, Cookie, etc.). + * + * @param redactedKeys list of regex patterns. + * @return the builder instance. + */ + public ConfigBuilder redactedKeys(List redactedKeys) { + this.redactedKeys = redactedKeys; + return this; + } + + /** + * URL sanitizer applied to the request URL before the payload is sent. + * Defaults to {@link DefaultUrlSanitizer#INSTANCE}. + * + * @param urlSanitizer the sanitizer. + * @return the builder instance. + */ + public ConfigBuilder urlSanitizer(StringUrlSanitizer urlSanitizer) { + this.urlSanitizer = urlSanitizer; + return this; + } + /** * Builds the {@link Config config}. * @@ -584,6 +615,8 @@ private static class ConfigImpl implements Config { private final boolean compressPayload; private final int maximumTelemetryData; private final TelemetryEventTracker telemetryEventTracker; + private final List redactedKeys; + private final StringUrlSanitizer urlSanitizer; ConfigImpl(ConfigBuilder builder) { this.accessToken = builder.accessToken; @@ -619,6 +652,10 @@ private static class ConfigImpl implements Config { this.compressPayload = builder.compressPayload; this.maximumTelemetryData = builder.maximumTelemetryData; this.telemetryEventTracker = builder.telemetryEventTracker; + this.redactedKeys = builder.redactedKeys != null + ? builder.redactedKeys : Collections.emptyList(); + this.urlSanitizer = builder.urlSanitizer != null + ? builder.urlSanitizer : DefaultUrlSanitizer.INSTANCE; } @Override @@ -775,5 +812,15 @@ public int maximumTelemetryData() { public TelemetryEventTracker telemetryEventTracker() { return this.telemetryEventTracker; } + + @Override + public List redactedKeys() { + return redactedKeys; + } + + @Override + public StringUrlSanitizer urlSanitizer() { + return urlSanitizer; + } } }