Skip to content

Commit 93cca38

Browse files
adinaueradinauer
authored
Merge pull request #5834 from getsentry/fix/data-collection-spring-binding
fix(spring): [Data Collection 18] Bind key-value policies
2 parents 764db2e + 0664aae commit 93cca38

8 files changed

Lines changed: 145 additions & 11 deletions

File tree

‎sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryAutoConfigurationTest.kt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import io.sentry.IProfileConverter
1212
import io.sentry.IScopes
1313
import io.sentry.ITransportFactory
1414
import io.sentry.Integration
15+
import io.sentry.KeyValueCollectionBehavior
1516
import io.sentry.NoOpContinuousProfiler
1617
import io.sentry.NoOpProfileConverter
1718
import io.sentry.NoOpTransportFactory
@@ -306,6 +307,35 @@ class SentryAutoConfigurationTest {
306307
}
307308
}
308309

310+
@Test
311+
fun `data collection key value properties are applied to SentryOptions`() {
312+
contextRunner
313+
.withPropertyValues(
314+
"sentry.dsn=http://key@localhost/proj",
315+
"sentry.data-collection.cookies.mode=off",
316+
"sentry.data-collection.url-query-params.mode=allow-list",
317+
"sentry.data-collection.url-query-params.terms=page,sort",
318+
"sentry.data-collection.http-headers.request.mode=deny-list",
319+
"sentry.data-collection.http-headers.request.terms=forwarded,-ip",
320+
"sentry.data-collection.http-headers.response.mode=allow-list",
321+
"sentry.data-collection.http-headers.response.terms=content-type,x-request-id",
322+
)
323+
.run {
324+
val dataCollection = it.getBean(SentryProperties::class.java).dataCollection
325+
assertThat(dataCollection.cookies!!.mode).isEqualTo(KeyValueCollectionBehavior.Mode.OFF)
326+
assertThat(dataCollection.urlQueryParams!!.mode)
327+
.isEqualTo(KeyValueCollectionBehavior.Mode.ALLOW_LIST)
328+
assertThat(dataCollection.urlQueryParams!!.terms).containsExactly("page", "sort")
329+
assertThat(dataCollection.httpHeaders.request!!.mode)
330+
.isEqualTo(KeyValueCollectionBehavior.Mode.DENY_LIST)
331+
assertThat(dataCollection.httpHeaders.request!!.terms).containsExactly("forwarded", "-ip")
332+
assertThat(dataCollection.httpHeaders.response!!.mode)
333+
.isEqualTo(KeyValueCollectionBehavior.Mode.ALLOW_LIST)
334+
assertThat(dataCollection.httpHeaders.response!!.terms)
335+
.containsExactly("content-type", "x-request-id")
336+
}
337+
}
338+
309339
@Test
310340
fun `when tracePropagationTargets are not set, default is returned`() {
311341
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj").run {

‎sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import io.sentry.IProfileConverter
1313
import io.sentry.IScopes
1414
import io.sentry.ITransportFactory
1515
import io.sentry.Integration
16+
import io.sentry.KeyValueCollectionBehavior
1617
import io.sentry.NoOpContinuousProfiler
1718
import io.sentry.NoOpProfileConverter
1819
import io.sentry.NoOpTransportFactory
@@ -314,6 +315,35 @@ class SentryAutoConfigurationTest {
314315
}
315316
}
316317

318+
@Test
319+
fun `data collection key value properties are applied to SentryOptions`() {
320+
contextRunner
321+
.withPropertyValues(
322+
"sentry.dsn=http://key@localhost/proj",
323+
"sentry.data-collection.cookies.mode=off",
324+
"sentry.data-collection.url-query-params.mode=allow-list",
325+
"sentry.data-collection.url-query-params.terms=page,sort",
326+
"sentry.data-collection.http-headers.request.mode=deny-list",
327+
"sentry.data-collection.http-headers.request.terms=forwarded,-ip",
328+
"sentry.data-collection.http-headers.response.mode=allow-list",
329+
"sentry.data-collection.http-headers.response.terms=content-type,x-request-id",
330+
)
331+
.run {
332+
val dataCollection = it.getBean(SentryProperties::class.java).dataCollection
333+
assertThat(dataCollection.cookies!!.mode).isEqualTo(KeyValueCollectionBehavior.Mode.OFF)
334+
assertThat(dataCollection.urlQueryParams!!.mode)
335+
.isEqualTo(KeyValueCollectionBehavior.Mode.ALLOW_LIST)
336+
assertThat(dataCollection.urlQueryParams!!.terms).containsExactly("page", "sort")
337+
assertThat(dataCollection.httpHeaders.request!!.mode)
338+
.isEqualTo(KeyValueCollectionBehavior.Mode.DENY_LIST)
339+
assertThat(dataCollection.httpHeaders.request!!.terms).containsExactly("forwarded", "-ip")
340+
assertThat(dataCollection.httpHeaders.response!!.mode)
341+
.isEqualTo(KeyValueCollectionBehavior.Mode.ALLOW_LIST)
342+
assertThat(dataCollection.httpHeaders.response!!.terms)
343+
.containsExactly("content-type", "x-request-id")
344+
}
345+
}
346+
317347
@Test
318348
fun `when tracePropagationTargets are not set, default is returned`() {
319349
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj").run {

‎sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import io.sentry.IProfileConverter
1313
import io.sentry.IScopes
1414
import io.sentry.ITransportFactory
1515
import io.sentry.Integration
16+
import io.sentry.KeyValueCollectionBehavior
1617
import io.sentry.NoOpContinuousProfiler
1718
import io.sentry.NoOpProfileConverter
1819
import io.sentry.NoOpTransportFactory
@@ -312,6 +313,35 @@ class SentryAutoConfigurationTest {
312313
}
313314
}
314315

316+
@Test
317+
fun `data collection key value properties are applied to SentryOptions`() {
318+
contextRunner
319+
.withPropertyValues(
320+
"sentry.dsn=http://key@localhost/proj",
321+
"sentry.data-collection.cookies.mode=off",
322+
"sentry.data-collection.url-query-params.mode=allow-list",
323+
"sentry.data-collection.url-query-params.terms=page,sort",
324+
"sentry.data-collection.http-headers.request.mode=deny-list",
325+
"sentry.data-collection.http-headers.request.terms=forwarded,-ip",
326+
"sentry.data-collection.http-headers.response.mode=allow-list",
327+
"sentry.data-collection.http-headers.response.terms=content-type,x-request-id",
328+
)
329+
.run {
330+
val dataCollection = it.getBean(SentryProperties::class.java).dataCollection
331+
assertThat(dataCollection.cookies!!.mode).isEqualTo(KeyValueCollectionBehavior.Mode.OFF)
332+
assertThat(dataCollection.urlQueryParams!!.mode)
333+
.isEqualTo(KeyValueCollectionBehavior.Mode.ALLOW_LIST)
334+
assertThat(dataCollection.urlQueryParams!!.terms).containsExactly("page", "sort")
335+
assertThat(dataCollection.httpHeaders.request!!.mode)
336+
.isEqualTo(KeyValueCollectionBehavior.Mode.DENY_LIST)
337+
assertThat(dataCollection.httpHeaders.request!!.terms).containsExactly("forwarded", "-ip")
338+
assertThat(dataCollection.httpHeaders.response!!.mode)
339+
.isEqualTo(KeyValueCollectionBehavior.Mode.ALLOW_LIST)
340+
assertThat(dataCollection.httpHeaders.response!!.terms)
341+
.containsExactly("content-type", "x-request-id")
342+
}
343+
}
344+
315345
@Test
316346
fun `when tracePropagationTargets are not set, default is returned`() {
317347
dsnEnabledRunner.run {

‎sentry/api/sentry.api‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,13 +1457,16 @@ public abstract interface class io/sentry/JsonUnknown {
14571457
}
14581458

14591459
public final class io/sentry/KeyValueCollectionBehavior {
1460+
public fun <init> ()V
14601461
public static fun allowList ([Ljava/lang/String;)Lio/sentry/KeyValueCollectionBehavior;
14611462
public static fun denyList ([Ljava/lang/String;)Lio/sentry/KeyValueCollectionBehavior;
14621463
public fun equals (Ljava/lang/Object;)Z
14631464
public fun getMode ()Lio/sentry/KeyValueCollectionBehavior$Mode;
14641465
public fun getTerms ()Ljava/util/List;
14651466
public fun hashCode ()I
14661467
public static fun off ()Lio/sentry/KeyValueCollectionBehavior;
1468+
public fun setMode (Lio/sentry/KeyValueCollectionBehavior$Mode;)V
1469+
public fun setTerms (Ljava/util/List;)V
14671470
}
14681471

14691472
public final class io/sentry/KeyValueCollectionBehavior$Mode : java/lang/Enum {

‎sentry/src/main/java/io/sentry/DataCollectionResolver.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@
99
@ApiStatus.Internal
1010
public final class DataCollectionResolver {
1111

12-
private static final @NotNull KeyValueCollectionBehavior OFF = KeyValueCollectionBehavior.off();
13-
private static final @NotNull KeyValueCollectionBehavior EMPTY_DENY_LIST =
14-
KeyValueCollectionBehavior.denyList();
15-
1612
private final @NotNull SentryOptions options;
1713

1814
DataCollectionResolver(final @NotNull SentryOptions options) {
@@ -71,9 +67,11 @@ public boolean isGraphqlVariablesWithLegacyAlways() {
7167
return cookies;
7268
}
7369
if (isDataCollectionConfigured()) {
74-
return EMPTY_DENY_LIST;
70+
return KeyValueCollectionBehavior.denyList();
7571
}
76-
return options.isSendDefaultPii() ? EMPTY_DENY_LIST : OFF;
72+
return options.isSendDefaultPii()
73+
? KeyValueCollectionBehavior.denyList()
74+
: KeyValueCollectionBehavior.off();
7775
}
7876

7977
public @NotNull KeyValueCollectionBehavior getUrlQueryParams() {
@@ -128,7 +126,7 @@ private boolean explicitOrDefault(
128126

129127
private @NotNull KeyValueCollectionBehavior explicitOrEmptyDenyList(
130128
final @Nullable KeyValueCollectionBehavior explicit) {
131-
return explicit != null ? explicit : EMPTY_DENY_LIST;
129+
return explicit != null ? explicit : KeyValueCollectionBehavior.denyList();
132130
}
133131

134132
private boolean isHttpBodyEnabled(

‎sentry/src/main/java/io/sentry/KeyValueCollectionBehavior.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@ public enum Mode {
2020
ALLOW_LIST
2121
}
2222

23-
private final @NotNull Mode mode;
24-
private final @NotNull List<String> terms;
23+
private @NotNull Mode mode = Mode.DENY_LIST;
24+
private @NotNull List<String> terms = Collections.emptyList();
25+
26+
/** Creates a behavior that collects values using the built-in sensitive deny-list. */
27+
public KeyValueCollectionBehavior() {}
2528

2629
private KeyValueCollectionBehavior(final @NotNull Mode mode, final @NotNull List<String> terms) {
27-
this.mode = mode;
28-
this.terms = Collections.unmodifiableList(new ArrayList<>(terms));
30+
setMode(mode);
31+
setTerms(terms);
2932
}
3033

3134
/** Disables collection of the category. */
@@ -53,10 +56,18 @@ private KeyValueCollectionBehavior(final @NotNull Mode mode, final @NotNull List
5356
return mode;
5457
}
5558

59+
public void setMode(final @NotNull Mode mode) {
60+
this.mode = mode;
61+
}
62+
5663
public @NotNull List<String> getTerms() {
5764
return terms;
5865
}
5966

67+
public void setTerms(final @NotNull List<String> terms) {
68+
this.terms = Collections.unmodifiableList(new ArrayList<>(terms));
69+
}
70+
6071
@Override
6172
public boolean equals(final Object other) {
6273
if (this == other) {

‎sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,17 @@ class DataCollectionResolverTest {
244244
assertThat(options.dataCollectionResolver.cookies).isEqualTo(KeyValueCollectionBehavior.off())
245245
}
246246

247+
@Test
248+
fun `mutating one fallback key-value behavior does not affect other getters`() {
249+
val resolver = SentryOptions().apply { dataCollection.setUserInfo(true) }.dataCollectionResolver
250+
251+
resolver.cookies.terms = listOf("custom-cookie")
252+
253+
assertThat(resolver.urlQueryParams.terms).doesNotContain("custom-cookie")
254+
assertThat(resolver.httpRequestHeaders.terms).doesNotContain("custom-cookie")
255+
assertThat(resolver.httpResponseHeaders.terms).doesNotContain("custom-cookie")
256+
}
257+
247258
@Test
248259
fun `cookies use default deny list when unset and sendDefaultPii is true`() {
249260
val options = SentryOptions().apply { isSendDefaultPii = true }

‎sentry/src/test/java/io/sentry/KeyValueCollectionBehaviorTest.kt‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,27 @@ import com.google.common.truth.Truth.assertThat
44
import kotlin.test.Test
55

66
class KeyValueCollectionBehaviorTest {
7+
@Test
8+
fun `default constructor uses deny list with no terms`() {
9+
val behavior = KeyValueCollectionBehavior()
10+
11+
assertThat(behavior.mode).isEqualTo(KeyValueCollectionBehavior.Mode.DENY_LIST)
12+
assertThat(behavior.terms).isEmpty()
13+
}
14+
15+
@Test
16+
fun `setters update mode and defensively copy terms`() {
17+
val terms = mutableListOf("token")
18+
val behavior = KeyValueCollectionBehavior()
19+
20+
behavior.mode = KeyValueCollectionBehavior.Mode.ALLOW_LIST
21+
behavior.terms = terms
22+
terms[0] = "password"
23+
24+
assertThat(behavior.mode).isEqualTo(KeyValueCollectionBehavior.Mode.ALLOW_LIST)
25+
assertThat(behavior.terms).containsExactly("token")
26+
}
27+
728
@Test
829
fun `off has no terms`() {
930
val behavior = KeyValueCollectionBehavior.off()

0 commit comments

Comments
 (0)