From 8c1b6c0203dda5b69e71da2a5244c49802d0fb01 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 24 Sep 2026 14:36:08 +0200 Subject: [PATCH 1/6] test: pin fail-closed blocking behavior ahead of #12601's overload migration PR #12601 (APPSEC-70201) migrates ~59 AppSec blocking call sites from the (TraceSegment, RBA) overload to the (RequestContext, RBA) default added by #12519. This is a mechanical no-behavior-change migration for nearly all sites, but it currently lacks test coverage in several modules, so a silent regression there would only surface as a production incident. Adds unit and integration tests that pin the current fail-closed blocking behavior across the untested surface, so any regression introduced by the overload migration is caught by CI instead of production: - ProcessImplInstrumentationHelpers (CMDI/SHI RASP) - FileIORaspHelper (LFI RASP) and URLSinkCallSite (SSRF RASP) - AppSecEventTracker.dispatch() - StoredCharBody.maybeNotifyAndBlock() (migrated from Groovy to Java) - BlockResponseFunction's (TraceSegment, RBA) overload directly - play-appsec-common PathExtractionHelpers (deliberate tripwire: documents the fail-closed behavior expected to flip after #12601's rebase) - Play 2.5/2.6 ResultsStatus/StatusHeader advices and BodyParserHelpers - vertx-web RoutingContextSessionAdvice, FileUploadHelper and the RoutingContextJsonResponseAdvice BODY_JSON integration path - jax-rs-annotations-2.0 / jakarta-rs-annotations-3.0 MessageBodyWriter - Ratpack JsonRendererAdvice BODY_JSON integration path Also fixes FileIORaspHelperBlockingTest flakiness caused by pre-existing Groovy specs in java-io-1.8 leaking a mocked FileIORaspHelper.INSTANCE across the shared test JVM: the new test now saves and restores the real singleton around each test case via reflection. --- .../appsec/user/AppSecEventTrackerTest.java | 55 +++++ .../java/java-io-1.8/build.gradle | 1 + .../java/java-io-1.8/gradle.lockfile | 3 +- .../lang/FileIORaspHelperBlockingTest.java | 191 +++++++++++++++++ .../java/java-net/java-net-1.8/build.gradle | 1 + .../java-net/java-net-1.8/gradle.lockfile | 3 +- .../java/net/URLSinkCallSiteBlockingTest.java | 136 ++++++++++++ .../play/play-appsec-2.5/build.gradle | 2 + .../play/play-appsec-2.5/gradle.lockfile | 3 +- .../play25/server/PlayServerTest.groovy | 36 ++++ .../play25/appsec/BodyParserHelpersTest.java | 177 +++++++++++++++ .../JsonResponseBlockingAdviceTest.java | 168 +++++++++++++++ .../play/play-appsec-2.6/build.gradle | 2 + .../play/play-appsec-2.6/gradle.lockfile | 3 +- .../play26/server/PlayServerTest.groovy | 32 +++ .../play26/appsec/BodyParserHelpersTest.java | 201 ++++++++++++++++++ .../JsonResponseBlockingAdviceTest.java | 171 +++++++++++++++ .../play/play-appsec-common/build.gradle | 4 + .../play/play-appsec-common/gradle.lockfile | 3 +- .../appsec/PathExtractionHelpersTest.java | 199 +++++++++++++++++ .../server/RatpackHttpServerTest.groovy | 34 +++ .../jakarta-rs-annotations-3.0/build.gradle | 2 + .../gradle.lockfile | 3 +- .../jakarta3/MessageBodyWriterAdviceTest.java | 156 ++++++++++++++ .../jax-rs-annotations-2.0/build.gradle | 2 + .../jax-rs-annotations-2.0/gradle.lockfile | 3 +- .../jaxrs2/MessageBodyWriterAdviceTest.java | 156 ++++++++++++++ .../vertx-web/vertx-web-3.4/build.gradle | 1 + .../vertx-web/vertx-web-3.4/gradle.lockfile | 3 +- .../server/FileUploadHelperTest.java | 142 +++++++++++++ .../RoutingContextSessionAdviceTest.java | 147 +++++++++++++ .../vertx-web/vertx-web-4.0/build.gradle | 1 + .../vertx-web/vertx-web-4.0/gradle.lockfile | 3 +- .../server/VertxHttpServerForkedTest.groovy | 28 +++ .../server/FileUploadHelperTest.java | 142 +++++++++++++ .../RoutingContextSessionAdviceTest.java | 147 +++++++++++++ .../vertx-web/vertx-web-5.0/build.gradle | 2 + .../vertx-web/vertx-web-5.0/gradle.lockfile | 3 +- .../server/VertxHttpServerForkedTest.groovy | 27 +++ .../server/FileUploadHelperTest.java | 142 +++++++++++++ .../trace/api/http/StoredCharBodyTest.groovy | 77 ------- .../gateway/BlockResponseFunctionTest.java | 67 +++++- .../trace/api/http/StoredCharBodyTest.java | 159 ++++++++++++++ ...mplInstrumentationHelpersBlockingTest.java | 153 +++++++++++++ 44 files changed, 2901 insertions(+), 90 deletions(-) create mode 100644 dd-java-agent/instrumentation/java/java-io-1.8/src/test/java/datadog/trace/instrumentation/java/lang/FileIORaspHelperBlockingTest.java create mode 100644 dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/java/datadog/trace/instrumentation/java/net/URLSinkCallSiteBlockingTest.java create mode 100644 dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/JsonResponseBlockingAdviceTest.java create mode 100644 dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/JsonResponseBlockingAdviceTest.java create mode 100644 dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java create mode 100644 dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceTest.java create mode 100644 dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceTest.java create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperTest.java create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceTest.java create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperTest.java create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceTest.java create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperTest.java delete mode 100644 internal-api/src/test/groovy/datadog/trace/api/http/StoredCharBodyTest.groovy create mode 100644 internal-api/src/test/java/datadog/trace/api/http/StoredCharBodyTest.java create mode 100644 internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpersBlockingTest.java diff --git a/dd-java-agent/appsec/src/test/java/com/datadog/appsec/user/AppSecEventTrackerTest.java b/dd-java-agent/appsec/src/test/java/com/datadog/appsec/user/AppSecEventTrackerTest.java index 6f827e5f0b2..f6209c34f29 100644 --- a/dd-java-agent/appsec/src/test/java/com/datadog/appsec/user/AppSecEventTrackerTest.java +++ b/dd-java-agent/appsec/src/test/java/com/datadog/appsec/user/AppSecEventTrackerTest.java @@ -17,6 +17,7 @@ import static java.util.Collections.emptyMap; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -40,6 +41,7 @@ import datadog.trace.api.UserIdCollectionMode; import datadog.trace.api.appsec.AppSecEventTracker; import datadog.trace.api.function.TriFunction; +import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -62,6 +64,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.tabletest.junit.TableTest; @SuppressWarnings("deprecation") // exercises the deprecated v1 EventTracker API on purpose @@ -457,6 +461,27 @@ void blockingOnALogin() { () -> tracker.onLoginSuccessEvent(SDK, USER_LOGIN, USER_ID, METADATA)); } + @ParameterizedTest(name = "commit succeeds: {0}") + @ValueSource(booleans = {true, false}) + void blockingOnALoginCommitsBlockingResponse(boolean commitResult) { + Flow.Action.RequestBlockingAction action = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + when(loginEvent.apply(isA(RequestContext.class), eq(LOGIN_SUCCESS), eq(USER_LOGIN))) + .thenReturn(new ActionFlow<>(action)); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(requestContext.getBlockResponseFunction()).thenReturn(brf); + when(requestContext.getTraceSegment()).thenReturn(traceSegment); + + assertThrows( + BlockingException.class, + () -> tracker.onLoginSuccessEvent(SDK, USER_LOGIN, USER_ID, METADATA)); + + assertEquals(1, brf.calls); + assertSame(traceSegment, brf.lastSegment); + assertEquals(action.getStatusCode(), brf.lastStatusCode); + assertEquals(action.getBlockingContentType(), brf.lastTemplateType); + } + @Test void shouldNotFailOnNullCallback() { when(provider.getCallback(EVENTS.user())).thenReturn(null); @@ -575,6 +600,36 @@ public boolean isEnabled(UserIdCollectionMode mode) { } } + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + private RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return commitResult; + } + } + private static class ActionFlow implements Flow { private final Action action; diff --git a/dd-java-agent/instrumentation/java/java-io-1.8/build.gradle b/dd-java-agent/instrumentation/java/java-io-1.8/build.gradle index bbc1994c37f..de17621d11d 100644 --- a/dd-java-agent/instrumentation/java/java-io-1.8/build.gradle +++ b/dd-java-agent/instrumentation/java/java-io-1.8/build.gradle @@ -25,5 +25,6 @@ tasks.named("compileJava11TestGroovy", GroovyCompile) { dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:datadog:asm:iast-instrumenter') testImplementation group: 'org.apache.tomcat', name: 'tomcat-catalina', version: '9.0.56' + testImplementation libs.bundles.mockito java11TestImplementation sourceSets.test.output } diff --git a/dd-java-agent/instrumentation/java/java-io-1.8/gradle.lockfile b/dd-java-agent/instrumentation/java/java-io-1.8/gradle.lockfile index e1fb50049dd..32f09a2df6e 100644 --- a/dd-java-agent/instrumentation/java/java-io-1.8/gradle.lockfile +++ b/dd-java-agent/instrumentation/java/java-io-1.8/gradle.lockfile @@ -109,7 +109,8 @@ org.junit.platform:junit-platform-runner:1.14.1=java11TestRuntimeClasspath,lates org.junit.platform:junit-platform-suite-api:1.14.1=java11TestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=java11TestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=java11TestCompileClasspath,java11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=java11TestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=java11TestCompileClasspath,java11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=java11TestCompileClasspath,java11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=java11TestCompileClasspath,java11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=java11TestCompileClasspath,java11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/java/java-io-1.8/src/test/java/datadog/trace/instrumentation/java/lang/FileIORaspHelperBlockingTest.java b/dd-java-agent/instrumentation/java/java-io-1.8/src/test/java/datadog/trace/instrumentation/java/lang/FileIORaspHelperBlockingTest.java new file mode 100644 index 00000000000..5277a5909b8 --- /dev/null +++ b/dd-java-agent/instrumentation/java/java-io-1.8/src/test/java/datadog/trace/instrumentation/java/lang/FileIORaspHelperBlockingTest.java @@ -0,0 +1,191 @@ +package datadog.trace.instrumentation.java.lang; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.params.provider.Arguments.arguments; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.io.File; +import java.lang.reflect.Constructor; +import java.net.URI; +import java.util.Map; +import java.util.stream.Stream; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Pins the fail-closed blocking behavior of the LFI RASP check: when the AppSec callback returns a + * real {@link Flow.Action.RequestBlockingAction}, a {@link BlockingException} is always thrown from + * every public entry point, whether or not a {@link BlockResponseFunction} is present and + * regardless of whether it managed to commit the blocking response. + */ +class FileIORaspHelperBlockingTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private AgentTracer.TracerAPI originalTracer; + private FileIORaspHelper originalHelperInstance; + private TraceSegment traceSegment; + private RequestContext reqCtx; + + /** + * Several pre-existing Groovy specs in this module replace {@link FileIORaspHelper#INSTANCE} with + * a mock and never restore it, leaking a permanently-neutered singleton into every test that runs + * afterwards in the same test JVM. Force a genuine instance for the duration of this test + * regardless of execution order, and restore whatever was there before. + */ + private static FileIORaspHelper newRealInstance() throws ReflectiveOperationException { + Constructor ctor = FileIORaspHelper.class.getDeclaredConstructor(); + ctor.setAccessible(true); + return ctor.newInstance(); + } + + @BeforeEach + void setUp() throws ReflectiveOperationException { + originalHelperInstance = FileIORaspHelper.INSTANCE; + FileIORaspHelper.INSTANCE = newRealInstance(); + + originalTracer = AgentTracer.get(); + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + + @SuppressWarnings("unchecked") + Flow blockingFlow = mock(Flow.class); + when(blockingFlow.getAction()).thenReturn(RBA); + CallbackProvider callbackProvider = mock(CallbackProvider.class); + when(callbackProvider.getCallback(EVENTS.fileLoaded())).thenReturn((ctx, path) -> blockingFlow); + when(callbackProvider.getCallback(EVENTS.fileWritten())) + .thenReturn((ctx, path) -> blockingFlow); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.activeSpan()).thenReturn(span); + when(tracer.getCallbackProvider(RequestContextSlot.APPSEC)).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + FileIORaspHelper.INSTANCE = originalHelperInstance; + } + + static Stream entryPoints() { + // Read INSTANCE lazily inside each Executable, not here: @MethodSource is resolved by JUnit + // before @BeforeEach runs, so capturing INSTANCE at this point could still observe a mock + // leaked by an earlier Groovy spec in this module (see setUp()'s javadoc). + return Stream.of( + arguments( + "beforeFileLoaded(String)", + (Executable) () -> FileIORaspHelper.INSTANCE.beforeFileLoaded("f")), + arguments( + "beforeFileLoaded(String, String)", + (Executable) () -> FileIORaspHelper.INSTANCE.beforeFileLoaded("/tmp", "f")), + arguments( + "beforeFileLoaded(String, String[])", + (Executable) + () -> + FileIORaspHelper.INSTANCE.beforeFileLoaded("/tmp", new String[] {"log", "f"})), + arguments( + "beforeFileLoaded(File, String)", + (Executable) () -> FileIORaspHelper.INSTANCE.beforeFileLoaded(new File("/tmp"), "f")), + arguments( + "beforeFileLoaded(URI)", + (Executable) + () -> FileIORaspHelper.INSTANCE.beforeFileLoaded(URI.create("file:/tmp/f"))), + arguments( + "beforeFileWritten(String)", + (Executable) () -> FileIORaspHelper.INSTANCE.beforeFileWritten("f")), + arguments( + "beforeRandomAccessFileOpened(String, String)", + (Executable) () -> FileIORaspHelper.INSTANCE.beforeRandomAccessFileOpened("f", "rw"))); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("entryPoints") + void throwsWithoutBlockResponseFunction(String name, Executable entryPoint) { + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertThrows(BlockingException.class, entryPoint); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("entryPoints") + void throwsWhenBlockResponseFunctionCommits(String name, Executable entryPoint) { + assertThrowsAndCommits(entryPoint, true); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("entryPoints") + void throwsWhenBlockResponseFunctionFailsToCommit(String name, Executable entryPoint) { + assertThrowsAndCommits(entryPoint, false); + } + + private void assertThrowsAndCommits(Executable entryPoint, boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertThrows(BlockingException.class, entryPoint); + + brf.assertCommittedOnce(traceSegment); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + private RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return commitResult; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + // the first blocking event aborts the call, so a commit is attempted exactly once + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/build.gradle b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/build.gradle index 82dbf59affa..cc7e4380075 100644 --- a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/build.gradle +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/build.gradle @@ -14,6 +14,7 @@ addTestSuiteForDir('latestDepTest', 'test') dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:datadog:asm:iast-instrumenter') testImplementation group: 'org.springframework', name: 'spring-web', version: '4.3.7.RELEASE' + testImplementation libs.bundles.mockito } // IBM8 is having troubles with TLS set up by jetty 9.4. diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/gradle.lockfile b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/gradle.lockfile index c4efc68eb70..c9b96ba5076 100644 --- a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/gradle.lockfile +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/gradle.lockfile @@ -98,7 +98,8 @@ org.junit.platform:junit-platform-runner:1.14.1=latestDepTestRuntimeClasspath,te org.junit.platform:junit-platform-suite-api:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=latestDepTestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/java/datadog/trace/instrumentation/java/net/URLSinkCallSiteBlockingTest.java b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/java/datadog/trace/instrumentation/java/net/URLSinkCallSiteBlockingTest.java new file mode 100644 index 00000000000..8f2032f2d9c --- /dev/null +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/java/datadog/trace/instrumentation/java/net/URLSinkCallSiteBlockingTest.java @@ -0,0 +1,136 @@ +package datadog.trace.instrumentation.java.net; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.sink.SsrfModule; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Pins the fail-closed blocking behavior of the SSRF RASP check: when the AppSec callback returns a + * real {@link Flow.Action.RequestBlockingAction}, a {@link BlockingException} is always thrown, + * whether or not a {@link BlockResponseFunction} is present and regardless of whether it managed to + * commit the blocking response. + */ +class URLSinkCallSiteBlockingTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private AgentTracer.TracerAPI originalTracer; + private SsrfModule originalSsrfModule; + private TraceSegment traceSegment; + private RequestContext reqCtx; + private URL url; + + @BeforeEach + void setUp() throws MalformedURLException { + // keep the IAST side of the call site out of the way (a module left by another test would run) + originalSsrfModule = InstrumentationBridge.SSRF; + InstrumentationBridge.SSRF = null; + + originalTracer = AgentTracer.get(); + url = new URL("http://169.254.169.254/latest/meta-data/"); + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + + @SuppressWarnings("unchecked") + Flow blockingFlow = mock(Flow.class); + when(blockingFlow.getAction()).thenReturn(RBA); + CallbackProvider callbackProvider = mock(CallbackProvider.class); + when(callbackProvider.getCallback(EVENTS.httpClientRequest())) + .thenReturn((ctx, request) -> blockingFlow); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.activeSpan()).thenReturn(span); + when(tracer.getCallbackProvider(RequestContextSlot.APPSEC)).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + InstrumentationBridge.SSRF = originalSsrfModule; + } + + @Test + void throwsWithoutBlockResponseFunction() { + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertThrows(BlockingException.class, () -> URLSinkCallSite.beforeOpenConnection(url)); + } + + @ParameterizedTest(name = "commit succeeds: {0}") + @ValueSource(booleans = {true, false}) + void throwsWithBlockResponseFunction(boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertThrows(BlockingException.class, () -> URLSinkCallSite.beforeOpenConnection(url)); + + brf.assertCommittedOnce(traceSegment); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + private RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return commitResult; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/build.gradle b/dd-java-agent/instrumentation/play/play-appsec-2.5/build.gradle index 9969ff82256..b918bdc43bc 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/build.gradle +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/build.gradle @@ -74,6 +74,8 @@ dependencies { compileOnly group: 'com.typesafe.play', name: 'play_2.11', version: '2.5.0' implementation(project(':dd-java-agent:instrumentation:play:play-appsec-common')) + testImplementation libs.bundles.mockito + routeGeneratorImplementation libs.scala211 routeGeneratorImplementation group: 'com.typesafe.play', name: "routes-compiler_2.11", version: '2.5.0' diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/gradle.lockfile b/dd-java-agent/instrumentation/play/play-appsec-2.5/gradle.lockfile index f13d8380484..9e9f0ae27c1 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/gradle.lockfile +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/gradle.lockfile @@ -233,7 +233,8 @@ org.junit.platform:junit-platform-runner:1.14.1=latestDepTestRuntimeClasspath,te org.junit.platform:junit-platform-suite-api:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=latestDepTestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy index fd10be014f3..5f9465c913f 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy @@ -7,13 +7,18 @@ import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.play24.PlayHttpServerDecorator +import groovy.json.JsonOutput import groovy.transform.CompileStatic +import okhttp3.MediaType +import okhttp3.RequestBody import play.server.Server +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import static org.junit.jupiter.api.Assumptions.assumeTrue class PlayServerTest extends HttpServerTest { @@ -108,6 +113,37 @@ class PlayServerTest extends HttpServerTest { true } + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching StatusHeaderSendJsonAdvice (Java routers) or ResultsStatusApplyAdvice (Scala routers). + */ + def 'test blocking on json response body'() { + setup: + assumeTrue(testBlockingOnResponse() && testResponseBodyJson()) + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } + @Override String testPathParam() { '/path/?/param' diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpersTest.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpersTest.java index e14f55c7d74..4e221ea4d36 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpersTest.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/BodyParserHelpersTest.java @@ -1,21 +1,52 @@ package datadog.trace.instrumentation.play25.appsec; +import static datadog.trace.api.gateway.Events.EVENTS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.lang.reflect.Method; import java.math.BigDecimal; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import play.api.libs.json.JsValue; import play.api.mvc.MultipartFormData; +import scala.collection.JavaConverters; class BodyParserHelpersTest { + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + @AfterEach + void restoreTracer() { + AgentTracer.forceRegister(originalTracer); + } + private static JsValue parse(String json) { return play.api.libs.json.Json$.MODULE$.parse(json); } @@ -149,6 +180,152 @@ void collectFilenames_mixedPartsFiltered() throws Exception { assertEquals(Arrays.asList("a.pdf", "b.jpg"), result); } + // --- blocking tests: pin that a RequestBlockingAction returned by the AppSec callback is + // honored (commit attempted, BlockingException thrown only if the commit succeeds) --- + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void multipartFilenamesCallbackBlocks(boolean commitResult) throws Exception { + AppSecFixture fx = new AppSecFixture(commitResult); + fx.onFilenames((ctx, names) -> blockingFlow()); + fx.onFilesContent((ctx, contents) -> fx.contentCallbackInvoked()); + fx.register(); + MultipartFormData data = multipartFormData(filePart("f", "evil.php")); + + if (commitResult) { + BlockingException ex = + assertThrows( + BlockingException.class, + () -> BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + assertEquals("Blocked request (multipart file upload)", ex.getMessage()); + // files content inspection is skipped once a block is pending + assertEquals(0, fx.contentCallbackInvocations); + } else { + assertSame(data, BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + assertEquals(1, fx.contentCallbackInvocations); + } + fx.brf.assertCommittedOnce(fx.segment); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void multipartFilesContentCallbackBlocks(boolean commitResult) throws Exception { + AppSecFixture fx = new AppSecFixture(commitResult); + fx.onFilenames((ctx, names) -> Flow.ResultFlow.empty()); + fx.onFilesContent((ctx, contents) -> blockingFlow()); + fx.register(); + MultipartFormData data = multipartFormData(filePart("f", "evil.php")); + + if (commitResult) { + BlockingException ex = + assertThrows( + BlockingException.class, + () -> BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + assertEquals("Blocked request (multipart file upload content)", ex.getMessage()); + } else { + assertSame(data, BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + } + fx.brf.assertCommittedOnce(fx.segment); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + @SafeVarargs + @SuppressWarnings("unchecked") + private static MultipartFormData multipartFormData( + MultipartFormData.FilePart... files) { + return new MultipartFormData<>( + (scala.collection.immutable.Map>) + (Object) scala.collection.immutable.Map$.MODULE$.empty(), + JavaConverters.asScalaBufferConverter(Arrays.asList(files)).asScala(), + JavaConverters.asScalaBufferConverter(Collections.emptyList()) + .asScala()); + } + + /** Wires an active span with AppSec request data and stubs the AppSec gateway callbacks. */ + private static final class AppSecFixture { + final TraceSegment segment = mock(TraceSegment.class); + final RecordingBlockResponseFunction brf; + final RequestContext reqCtx = mock(RequestContext.class); + final CallbackProvider cbp = mock(CallbackProvider.class); + int contentCallbackInvocations; + + AppSecFixture(boolean commitResult) { + brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getTraceSegment()).thenReturn(segment); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + } + + void onFilenames(BiFunction, Flow> cb) { + when(cbp.getCallback(EVENTS.requestFilesFilenames())).thenReturn(cb); + } + + void onFilesContent(BiFunction, Flow> cb) { + when(cbp.getCallback(EVENTS.requestFilesContent())).thenReturn(cb); + } + + Flow contentCallbackInvoked() { + contentCallbackInvocations++; + return Flow.ResultFlow.empty(); + } + + void register() { + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(cbp); + when(tracer.activeSpan()).thenReturn(span); + AgentTracer.forceRegister(tracer); + } + } + + /** Fake implementing only the abstract 5-arg method, so any default overload routes here. */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int invocations; + private TraceSegment segment; + private int statusCode; + private BlockingContentType templateType; + + RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + invocations++; + this.segment = segment; + this.statusCode = statusCode; + this.templateType = templateType; + return commitResult; + } + + void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, invocations); + assertSame(expectedSegment, segment); + assertEquals(403, statusCode); + assertEquals(BlockingContentType.AUTO, templateType); + } + } + @SuppressWarnings("unchecked") private static MultipartFormData.FilePart filePart(String key, String filename) throws Exception { diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/JsonResponseBlockingAdviceTest.java b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/JsonResponseBlockingAdviceTest.java new file mode 100644 index 00000000000..906020e6ff9 --- /dev/null +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/java/datadog/trace/instrumentation/play25/appsec/JsonResponseBlockingAdviceTest.java @@ -0,0 +1,168 @@ +package datadog.trace.instrumentation.play25.appsec; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.databind.node.ObjectNode; +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import play.api.libs.json.JsValue; +import play.api.libs.json.Json$; +import play.mvc.StatusHeader; + +/** + * Pins the CURRENT behavior of {@link ResultsStatusApplyAdvice} and {@link + * StatusHeaderSendJsonAdvice}: when the AppSec response body callback returns a {@link + * Flow.Action.RequestBlockingAction} and a {@link BlockResponseFunction} is present, the advice + * attempts to commit the blocking response and then throws a {@link BlockingException} + * unconditionally, ignoring the commit result. + * + *

Deliberate tripwire: PR #12601 (APPSEC-70201) intentionally changes these advices to + * throw only when the commit succeeds. The {@code commitResult = false} cases are EXPECTED to fail + * once that PR is rebased on top of this test. That failure is the signal to consciously review the + * behavior change, not a regression to fix by reverting #12601. + */ +class JsonResponseBlockingAdviceTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + private RequestContext reqCtx; + private TraceSegment segment; + + @BeforeEach + void setup() { + segment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(segment); + // @RequiresRequestContext rewrites the compiled advice to read the request context from the + // active span and to bail out unless AppSec data is present + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + + BiFunction> callback = (ctx, body) -> blockingFlow(); + CallbackProvider cbp = mock(CallbackProvider.class); + when(cbp.getCallback(EVENTS.responseBody())).thenReturn(callback); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(cbp); + when(tracer.activeSpan()).thenReturn(span); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void resultsStatusApplyThrowsRegardlessOfCommitResult(boolean commitResult) { + RecordingBlockResponseFunction brf = givenBlockResponseFunction(commitResult); + JsValue content = Json$.MODULE$.parse("{\"key\":\"value\"}"); + + BlockingException ex = + assertThrows( + BlockingException.class, () -> ResultsStatusApplyAdvice.before(content, reqCtx)); + + assertEquals("Blocked request (for Results$Status/apply)", ex.getMessage()); + brf.assertCommittedOnce(segment); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void statusHeaderSendJsonThrowsRegardlessOfCommitResult(boolean commitResult) { + RecordingBlockResponseFunction brf = givenBlockResponseFunction(commitResult); + ObjectNode json = JsonNodeFactory.instance.objectNode().put("key", "value"); + + BlockingException ex; + try { + ex = + assertThrows( + BlockingException.class, () -> StatusHeaderSendJsonAdvice.before(json, reqCtx)); + } finally { + // reset the call depth incremented by before() + CallDepthThreadLocalMap.decrementCallDepth(StatusHeader.class); + } + + assertEquals("Blocked request (for StatusHeader/sendJson)", ex.getMessage()); + brf.assertCommittedOnce(segment); + } + + private RecordingBlockResponseFunction givenBlockResponseFunction(boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + return brf; + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + /** Fake implementing only the abstract 5-arg method, so any default overload routes here. */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int invocations; + private TraceSegment segment; + private int statusCode; + private BlockingContentType templateType; + + RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + invocations++; + this.segment = segment; + this.statusCode = statusCode; + this.templateType = templateType; + return commitResult; + } + + void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, invocations); + assertSame(expectedSegment, segment); + assertEquals(403, statusCode); + assertEquals(BlockingContentType.AUTO, templateType); + } + } +} diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/build.gradle b/dd-java-agent/instrumentation/play/play-appsec-2.6/build.gradle index 1a9b2ecd168..affff01e2a1 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/build.gradle +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/build.gradle @@ -33,6 +33,8 @@ dependencies { compileOnly project(':dd-java-agent:instrumentation:play:play-2.6') implementation(project(':dd-java-agent:instrumentation:play:play-appsec-common')) + testImplementation libs.bundles.mockito + testImplementation project(':dd-java-agent:instrumentation:play:play-2.6') testImplementation testFixtures(project(':dd-java-agent:instrumentation:play:play-2.6')) testImplementation group: 'com.typesafe.play', name: "play-java_2.11", version: '2.6.0' diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/gradle.lockfile b/dd-java-agent/instrumentation/play/play-appsec-2.6/gradle.lockfile index 836ee9ef36a..713bc638bfd 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/gradle.lockfile +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/gradle.lockfile @@ -169,7 +169,8 @@ org.junit.platform:junit-platform-runner:1.14.1=testRuntimeClasspath org.junit.platform:junit-platform-suite-api:1.14.1=testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=testRuntimeClasspath org.junit:junit-bom:5.14.1=testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=testRuntimeClasspath +org.mockito:mockito-core:4.4.0=testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy index 0ac729a38ac..e889a3122ad 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy @@ -1,8 +1,10 @@ package datadog.trace.instrumentation.play26.server +import groovy.json.JsonOutput import okhttp3.MediaType import okhttp3.RequestBody +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_XML class PlayServerTest extends AbstractPlayServerTest { @@ -17,6 +19,36 @@ class PlayServerTest extends AbstractPlayServerTest { true } + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching StatusHeaderSendJsonAdvice. + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } + def 'test instrumentation gateway xml request body'() { setup: def request = request( diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpersTest.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpersTest.java index 17fd88344a1..05d1956be92 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpersTest.java +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpersTest.java @@ -1,21 +1,54 @@ package datadog.trace.instrumentation.play26.appsec; +import static datadog.trace.api.gateway.Events.EVENTS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.lang.reflect.Method; import java.math.BigDecimal; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import play.api.libs.json.JsValue; import play.api.mvc.MultipartFormData; +import scala.collection.JavaConverters; +import scala.xml.NodeSeq; +import scala.xml.XML$; class BodyParserHelpersTest { + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + @AfterEach + void restoreTracer() { + AgentTracer.forceRegister(originalTracer); + } + private static JsValue parse(String json) { return play.api.libs.json.Json$.MODULE$.parse(json); } @@ -150,6 +183,174 @@ void collectFilenames_mixedPartsFiltered() throws Exception { assertEquals(Arrays.asList("a.pdf", "b.jpg"), result); } + // --- blocking tests: pin that a RequestBlockingAction returned by the AppSec callback is + // honored (commit attempted, BlockingException thrown only if the commit succeeds) --- + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void multipartFilenamesCallbackBlocks(boolean commitResult) throws Exception { + AppSecFixture fx = new AppSecFixture(commitResult); + fx.onFilenames((ctx, names) -> blockingFlow()); + fx.onFilesContent((ctx, contents) -> fx.contentCallbackInvoked()); + fx.register(); + MultipartFormData data = multipartFormData(filePart("f", "evil.php")); + + if (commitResult) { + BlockingException ex = + assertThrows( + BlockingException.class, + () -> BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + assertEquals("Blocked request (multipart file upload)", ex.getMessage()); + // files content inspection is skipped once a block is pending + assertEquals(0, fx.contentCallbackInvocations); + } else { + assertSame(data, BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + assertEquals(1, fx.contentCallbackInvocations); + } + fx.brf.assertCommittedOnce(fx.segment); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void multipartFilesContentCallbackBlocks(boolean commitResult) throws Exception { + AppSecFixture fx = new AppSecFixture(commitResult); + fx.onFilenames((ctx, names) -> Flow.ResultFlow.empty()); + fx.onFilesContent((ctx, contents) -> blockingFlow()); + fx.register(); + MultipartFormData data = multipartFormData(filePart("f", "evil.php")); + + if (commitResult) { + BlockingException ex = + assertThrows( + BlockingException.class, + () -> BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + assertEquals("Blocked request (multipart file upload content)", ex.getMessage()); + } else { + assertSame(data, BodyParserHelpers.getHandleMultipartFormDataF().apply(data)); + } + fx.brf.assertCommittedOnce(fx.segment); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void xmlBodyCallbackBlocks(boolean commitResult) { + AppSecFixture fx = new AppSecFixture(commitResult); + fx.onRequestBody((ctx, body) -> blockingFlow()); + fx.register(); + NodeSeq xml = XML$.MODULE$.loadString("text"); + + if (commitResult) { + BlockingException ex = + assertThrows(BlockingException.class, () -> BodyParserHelpers.getHandleXmlF().apply(xml)); + assertEquals("Blocked request (for xml)", ex.getMessage()); + } else { + assertSame(xml, BodyParserHelpers.getHandleXmlF().apply(xml)); + } + fx.brf.assertCommittedOnce(fx.segment); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + @SafeVarargs + @SuppressWarnings("unchecked") + private static MultipartFormData multipartFormData( + MultipartFormData.FilePart... files) { + return new MultipartFormData<>( + (scala.collection.immutable.Map>) + (Object) scala.collection.immutable.Map$.MODULE$.empty(), + JavaConverters.asScalaBufferConverter(Arrays.asList(files)).asScala(), + JavaConverters.asScalaBufferConverter(Collections.emptyList()) + .asScala()); + } + + /** Wires an active span with AppSec request data and stubs the AppSec gateway callbacks. */ + private static final class AppSecFixture { + final TraceSegment segment = mock(TraceSegment.class); + final RecordingBlockResponseFunction brf; + final RequestContext reqCtx = mock(RequestContext.class); + final CallbackProvider cbp = mock(CallbackProvider.class); + int contentCallbackInvocations; + + AppSecFixture(boolean commitResult) { + brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getTraceSegment()).thenReturn(segment); + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + } + + void onFilenames(BiFunction, Flow> cb) { + when(cbp.getCallback(EVENTS.requestFilesFilenames())).thenReturn(cb); + } + + void onFilesContent(BiFunction, Flow> cb) { + when(cbp.getCallback(EVENTS.requestFilesContent())).thenReturn(cb); + } + + void onRequestBody(BiFunction> cb) { + when(cbp.getCallback(EVENTS.requestBodyProcessed())).thenReturn(cb); + } + + Flow contentCallbackInvoked() { + contentCallbackInvocations++; + return Flow.ResultFlow.empty(); + } + + void register() { + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(cbp); + when(tracer.activeSpan()).thenReturn(span); + AgentTracer.forceRegister(tracer); + } + } + + /** Fake implementing only the abstract 5-arg method, so any default overload routes here. */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int invocations; + private TraceSegment segment; + private int statusCode; + private BlockingContentType templateType; + + RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + invocations++; + this.segment = segment; + this.statusCode = statusCode; + this.templateType = templateType; + return commitResult; + } + + void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, invocations); + assertSame(expectedSegment, segment); + assertEquals(403, statusCode); + assertEquals(BlockingContentType.AUTO, templateType); + } + } + @SuppressWarnings("unchecked") private static MultipartFormData.FilePart filePart(String key, String filename) throws Exception { diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/JsonResponseBlockingAdviceTest.java b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/JsonResponseBlockingAdviceTest.java new file mode 100644 index 00000000000..46c2afd4cd9 --- /dev/null +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/java/datadog/trace/instrumentation/play26/appsec/JsonResponseBlockingAdviceTest.java @@ -0,0 +1,171 @@ +package datadog.trace.instrumentation.play26.appsec; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.databind.node.ObjectNode; +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.instrumentation.play26.appsec.ResultsStatusInstrumentation.ResultsStatusApplyAdvice; +import datadog.trace.instrumentation.play26.appsec.StatusHeaderInstrumentation.StatusHeaderSendJsonAdvice; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import play.api.libs.json.JsValue; +import play.api.libs.json.Json$; +import play.mvc.StatusHeader; + +/** + * Pins the CURRENT behavior of {@link ResultsStatusApplyAdvice} and {@link + * StatusHeaderSendJsonAdvice} (also applied to Play 2.7+ via the {@code play26Plus} muzzle + * directive): when the AppSec response body callback returns a {@link + * Flow.Action.RequestBlockingAction} and a {@link BlockResponseFunction} is present, the advice + * attempts to commit the blocking response and then throws a {@link BlockingException} + * unconditionally, ignoring the commit result. + * + *

Deliberate tripwire: PR #12601 (APPSEC-70201) intentionally changes these advices to + * throw only when the commit succeeds. The {@code commitResult = false} cases are EXPECTED to fail + * once that PR is rebased on top of this test. That failure is the signal to consciously review the + * behavior change, not a regression to fix by reverting #12601. + */ +class JsonResponseBlockingAdviceTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + private RequestContext reqCtx; + private TraceSegment segment; + + @BeforeEach + void setup() { + segment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(segment); + // @RequiresRequestContext rewrites the compiled advice to read the request context from the + // active span and to bail out unless AppSec data is present + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + + BiFunction> callback = (ctx, body) -> blockingFlow(); + CallbackProvider cbp = mock(CallbackProvider.class); + when(cbp.getCallback(EVENTS.responseBody())).thenReturn(callback); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(cbp); + when(tracer.activeSpan()).thenReturn(span); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void resultsStatusApplyThrowsRegardlessOfCommitResult(boolean commitResult) { + RecordingBlockResponseFunction brf = givenBlockResponseFunction(commitResult); + JsValue content = Json$.MODULE$.parse("{\"key\":\"value\"}"); + + BlockingException ex = + assertThrows( + BlockingException.class, () -> ResultsStatusApplyAdvice.after(content, reqCtx)); + + assertEquals("Blocked request (for Results$Status/apply)", ex.getMessage()); + brf.assertCommittedOnce(segment); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void statusHeaderSendJsonThrowsRegardlessOfCommitResult(boolean commitResult) { + RecordingBlockResponseFunction brf = givenBlockResponseFunction(commitResult); + ObjectNode json = JsonNodeFactory.instance.objectNode().put("key", "value"); + + BlockingException ex; + try { + ex = + assertThrows( + BlockingException.class, () -> StatusHeaderSendJsonAdvice.before(json, reqCtx)); + } finally { + // reset the call depth incremented by before() + CallDepthThreadLocalMap.decrementCallDepth(StatusHeader.class); + } + + assertEquals("Blocked request (for StatusHeader/sendJson)", ex.getMessage()); + brf.assertCommittedOnce(segment); + } + + private RecordingBlockResponseFunction givenBlockResponseFunction(boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + return brf; + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + /** Fake implementing only the abstract 5-arg method, so any default overload routes here. */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int invocations; + private TraceSegment segment; + private int statusCode; + private BlockingContentType templateType; + + RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + invocations++; + this.segment = segment; + this.statusCode = statusCode; + this.templateType = templateType; + return commitResult; + } + + void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, invocations); + assertSame(expectedSegment, segment); + assertEquals(403, statusCode); + assertEquals(BlockingContentType.AUTO, templateType); + } + } +} diff --git a/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle b/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle index 3d48cbcff41..804fba845f1 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle +++ b/dd-java-agent/instrumentation/play/play-appsec-common/build.gradle @@ -1,3 +1,7 @@ plugins { id 'dd-trace-java.module.instrumentation' } + +dependencies { + testImplementation libs.bundles.mockito +} diff --git a/dd-java-agent/instrumentation/play/play-appsec-common/gradle.lockfile b/dd-java-agent/instrumentation/play/play-appsec-common/gradle.lockfile index bc77343b2bf..de4b074329f 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-common/gradle.lockfile +++ b/dd-java-agent/instrumentation/play/play-appsec-common/gradle.lockfile @@ -97,7 +97,8 @@ org.junit.platform:junit-platform-runner:1.14.1=testRuntimeClasspath org.junit.platform:junit-platform-suite-api:1.14.1=testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=testRuntimeClasspath org.junit:junit-bom:5.14.1=testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=testRuntimeClasspath +org.mockito:mockito-core:4.4.0=testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java b/dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java new file mode 100644 index 00000000000..328dc11daf7 --- /dev/null +++ b/dd-java-agent/instrumentation/play/play-appsec-common/src/test/java/datadog/trace/instrumentation/play/appsec/PathExtractionHelpersTest.java @@ -0,0 +1,199 @@ +package datadog.trace.instrumentation.play.appsec; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Pins the CURRENT fail-closed behavior of {@link + * PathExtractionHelpers#callRequestPathParamsCallback}: when the AppSec path params callback + * returns a {@link Flow.Action.RequestBlockingAction}, a non-null {@link BlockingException} is + * always returned, whether or not a {@link BlockResponseFunction} is present and regardless of + * whether committing the blocking response succeeds. + * + *

Deliberate tripwire: PR #12601 (APPSEC-70201) intentionally changes this to fail-open + * ({@code if (brf == null) return null;}) and ships its own {@code PathExtractionHelpersTest} at + * this same path. This test is EXPECTED to fail (or conflict) once that PR is rebased on top of it. + * That failure is the intended signal to consciously review the behavior change, not a regression + * to fix by reverting #12601. When rebasing, replace this file with the #12601 version once the + * fail-open change has been reviewed and accepted. + */ +class PathExtractionHelpersTest { + + private static final String ORIGIN = "test-origin"; + private static final Map PARAMS = singletonMap("id", "1"); + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + private RequestContext reqCtx; + private TraceSegment segment; + private CallbackProvider cbp; + + @BeforeEach + void setup() { + segment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(segment); + + cbp = mock(CallbackProvider.class); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(cbp); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + /** + * Tripwire: fail-closed without a BRF. #12601 makes this return null (fail-open), so this test is + * expected to fail after rebasing on #12601. + */ + @Test + void returnsBlockingExceptionWhenBlockingActionAndNoBlockResponseFunction() { + givenCallbackReturning(RBA); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + BlockingException result = call(PARAMS); + + assertNotNull(result); + assertEquals("Blocked request (for " + ORIGIN + ")", result.getMessage()); + } + + /** + * Fail-closed with a BRF: a {@link BlockingException} is returned and the commit is attempted, + * independently of the commit result. + */ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void returnsBlockingExceptionWhenBlockingActionRegardlessOfCommitResult(boolean commitResult) { + givenCallbackReturning(RBA); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + BlockingException result = call(PARAMS); + + assertNotNull(result); + assertEquals(1, brf.invocations); + assertSame(segment, brf.segment); + assertEquals(403, brf.statusCode); + assertEquals(BlockingContentType.AUTO, brf.templateType); + } + + @Test + void returnsNullWhenActionIsNotBlocking() { + givenCallbackReturning(Flow.Action.Noop.INSTANCE); + + assertNull(call(PARAMS)); + } + + @Test + void returnsNullWhenNoCallbackRegistered() { + when(cbp.getCallback(EVENTS.requestPathParams())).thenReturn(null); + + assertNull(call(PARAMS)); + } + + @Test + void returnsNullWhenCallbackThrows() { + BiFunction, Flow> callback = + (ctx, params) -> { + throw new IllegalStateException("boom"); + }; + when(cbp.getCallback(EVENTS.requestPathParams())).thenReturn(callback); + + assertNull(call(PARAMS)); + } + + @Test + void returnsNullForNullParams() { + givenCallbackReturning(RBA); + + assertNull(call(null)); + } + + @Test + void returnsNullForEmptyParams() { + givenCallbackReturning(RBA); + + assertNull(call(emptyMap())); + } + + private BlockingException call(Map params) { + return PathExtractionHelpers.callRequestPathParamsCallback(reqCtx, params, ORIGIN); + } + + private void givenCallbackReturning(Flow.Action action) { + BiFunction, Flow> callback = + (ctx, params) -> flowWith(action); + when(cbp.getCallback(EVENTS.requestPathParams())).thenReturn(callback); + } + + private static Flow flowWith(Flow.Action action) { + return new Flow() { + @Override + public Action getAction() { + return action; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + /** Fake implementing only the abstract 5-arg method, so any default overload routes here. */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + int invocations; + TraceSegment segment; + int statusCode; + BlockingContentType templateType; + + RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + invocations++; + this.segment = segment; + this.statusCode = statusCode; + this.templateType = templateType; + return commitResult; + } + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy index 0b665683b2b..80dd8f9d59b 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy @@ -8,8 +8,12 @@ import datadog.trace.api.DDTags import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator import datadog.trace.instrumentation.ratpack.RatpackServerDecorator +import groovy.json.JsonOutput +import okhttp3.MediaType +import okhttp3.RequestBody import ratpack.test.embed.EmbeddedApp +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED @@ -124,6 +128,36 @@ class RatpackHttpServerTest extends HttpServerTest { true } + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching JsonRendererAdvice. + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } + @Override void handlerSpan(TraceAssert trace, ServerEndpoint endpoint = SUCCESS) { trace.span { diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/build.gradle b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/build.gradle index 516456dd2da..9b3929ab4f4 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/build.gradle +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/build.gradle @@ -28,6 +28,8 @@ tasks.named("latestDepJava11Test", Test) { dependencies { compileOnly group: 'jakarta.ws.rs', name: 'jakarta.ws.rs-api', version: '3.0.0' + testImplementation libs.bundles.mockito + testImplementation project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0') testImplementation group: 'jakarta.ws.rs', name: 'jakarta.ws.rs-api', version: '3.0.0' testImplementation group: 'jakarta.xml.bind', name: 'jakarta.xml.bind-api', version: '3.0.0' diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/gradle.lockfile b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/gradle.lockfile index ba60392924a..b79a3ad8588 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/gradle.lockfile +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/gradle.lockfile @@ -103,7 +103,8 @@ org.junit.platform:junit-platform-runner:1.14.1=latestDepJava11TestRuntimeClassp org.junit.platform:junit-platform-suite-api:1.14.1=latestDepJava11TestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepJava11TestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=latestDepJava11TestCompileClasspath,latestDepJava11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=latestDepJava11TestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=latestDepJava11TestCompileClasspath,latestDepJava11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=latestDepJava11TestCompileClasspath,latestDepJava11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=latestDepJava11TestCompileClasspath,latestDepJava11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=latestDepJava11TestCompileClasspath,latestDepJava11TestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceTest.java b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceTest.java new file mode 100644 index 00000000000..ebec1eb2da2 --- /dev/null +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceTest.java @@ -0,0 +1,156 @@ +package datadog.trace.instrumentation.jakarta3; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.instrumentation.jakarta3.MessageBodyWriterInstrumentation.MessageBodyWriterAdvice; +import jakarta.ws.rs.core.MediaType; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Pins the current behavior of {@link MessageBodyWriterAdvice}: for an {@code application/json} + * entity, a {@link Flow.Action.RequestBlockingAction} returned by the AppSec response body callback + * makes the advice attempt to commit the blocking response and then throw a {@link + * BlockingException} regardless of the commit result. Non-JSON media types never reach the + * callback. + */ +class MessageBodyWriterAdviceTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + private RequestContext reqCtx; + private TraceSegment segment; + private int callbackInvocations; + private Object callbackEntity; + + @BeforeEach + void setup() { + segment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(segment); + // @RequiresRequestContext rewrites the compiled advice to read the request context from the + // active span and to bail out unless AppSec data is present + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + + BiFunction> callback = + (ctx, entity) -> { + callbackInvocations++; + callbackEntity = entity; + return blockingFlow(); + }; + CallbackProvider cbp = mock(CallbackProvider.class); + when(cbp.getCallback(EVENTS.responseBody())).thenReturn(callback); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(cbp); + when(tracer.activeSpan()).thenReturn(span); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void jsonEntityBlocksRegardlessOfCommitResult(boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + Object entity = new Object(); + + BlockingException ex = + assertThrows( + BlockingException.class, + () -> MessageBodyWriterAdvice.before(entity, MediaType.APPLICATION_JSON_TYPE, reqCtx)); + + assertEquals("Blocked request (for MessageBodyWriter)", ex.getMessage()); + assertEquals(1, callbackInvocations); + assertSame(entity, callbackEntity); + brf.assertCommittedOnce(segment); + } + + @Test + void nonJsonMediaTypeSkipsCallback() { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(true); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + MessageBodyWriterAdvice.before("plain body", MediaType.TEXT_PLAIN_TYPE, reqCtx); + + assertEquals(0, callbackInvocations); + assertEquals(0, brf.invocations); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + /** Fake implementing only the abstract 5-arg method, so any default overload routes here. */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int invocations; + private TraceSegment segment; + private int statusCode; + private BlockingContentType templateType; + + RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + invocations++; + this.segment = segment; + this.statusCode = statusCode; + this.templateType = templateType; + return commitResult; + } + + void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, invocations); + assertSame(expectedSegment, segment); + assertEquals(403, statusCode); + assertEquals(BlockingContentType.AUTO, templateType); + } + } +} diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/build.gradle b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/build.gradle index 90d4658e363..f8316d9033f 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/build.gradle +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/build.gradle @@ -28,6 +28,8 @@ addTestSuite('nestedTest') dependencies { compileOnly group: 'javax.ws.rs', name: 'javax.ws.rs-api', version: '2.0' + testImplementation libs.bundles.mockito + testImplementation project(':dd-java-agent:instrumentation:servlet:javax-servlet:javax-servlet-3.0') testImplementation project(':dd-java-agent:instrumentation:jersey:jersey-filter-2.0') testImplementation project(':dd-java-agent:instrumentation:resteasy:filter-resteasy:filter-resteasy-3.0') diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/gradle.lockfile b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/gradle.lockfile index 0aa2610f277..f7b21c8a784 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/gradle.lockfile +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/gradle.lockfile @@ -215,7 +215,8 @@ org.junit.platform:junit-platform-runner:1.14.1=latestDepTestRuntimeClasspath,ne org.junit.platform:junit-platform-suite-api:1.14.1=latestDepTestRuntimeClasspath,nestedTestRuntimeClasspath,resteasy31TestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepTestRuntimeClasspath,nestedTestRuntimeClasspath,resteasy31TestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,nestedTestCompileClasspath,nestedTestRuntimeClasspath,resteasy31TestCompileClasspath,resteasy31TestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=latestDepTestRuntimeClasspath,nestedTestRuntimeClasspath,resteasy31TestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,nestedTestCompileClasspath,nestedTestRuntimeClasspath,resteasy31TestCompileClasspath,resteasy31TestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,nestedTestCompileClasspath,nestedTestRuntimeClasspath,resteasy31TestCompileClasspath,resteasy31TestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,nestedTestCompileClasspath,nestedTestRuntimeClasspath,resteasy31TestCompileClasspath,resteasy31TestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,nestedTestCompileClasspath,nestedTestRuntimeClasspath,resteasy31TestCompileClasspath,resteasy31TestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceTest.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceTest.java new file mode 100644 index 00000000000..278b38f2f17 --- /dev/null +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceTest.java @@ -0,0 +1,156 @@ +package datadog.trace.instrumentation.jaxrs2; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.instrumentation.jaxrs2.MessageBodyWriterInstrumentation.MessageBodyWriterAdvice; +import java.util.Map; +import java.util.function.BiFunction; +import javax.ws.rs.core.MediaType; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Pins the current behavior of {@link MessageBodyWriterAdvice}: for an {@code application/json} + * entity, a {@link Flow.Action.RequestBlockingAction} returned by the AppSec response body callback + * makes the advice attempt to commit the blocking response and then throw a {@link + * BlockingException} regardless of the commit result. Non-JSON media types never reach the + * callback. + */ +class MessageBodyWriterAdviceTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + private RequestContext reqCtx; + private TraceSegment segment; + private int callbackInvocations; + private Object callbackEntity; + + @BeforeEach + void setup() { + segment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(segment); + // @RequiresRequestContext rewrites the compiled advice to read the request context from the + // active span and to bail out unless AppSec data is present + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + + BiFunction> callback = + (ctx, entity) -> { + callbackInvocations++; + callbackEntity = entity; + return blockingFlow(); + }; + CallbackProvider cbp = mock(CallbackProvider.class); + when(cbp.getCallback(EVENTS.responseBody())).thenReturn(callback); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))).thenReturn(cbp); + when(tracer.activeSpan()).thenReturn(span); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void jsonEntityBlocksRegardlessOfCommitResult(boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + Object entity = new Object(); + + BlockingException ex = + assertThrows( + BlockingException.class, + () -> MessageBodyWriterAdvice.before(entity, MediaType.APPLICATION_JSON_TYPE, reqCtx)); + + assertEquals("Blocked request (for MessageBodyWriter)", ex.getMessage()); + assertEquals(1, callbackInvocations); + assertSame(entity, callbackEntity); + brf.assertCommittedOnce(segment); + } + + @Test + void nonJsonMediaTypeSkipsCallback() { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(true); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + MessageBodyWriterAdvice.before("plain body", MediaType.TEXT_PLAIN_TYPE, reqCtx); + + assertEquals(0, callbackInvocations); + assertEquals(0, brf.invocations); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + /** Fake implementing only the abstract 5-arg method, so any default overload routes here. */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int invocations; + private TraceSegment segment; + private int statusCode; + private BlockingContentType templateType; + + RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + invocations++; + this.segment = segment; + this.statusCode = statusCode; + this.templateType = templateType; + return commitResult; + } + + void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, invocations); + assertSame(expectedSegment, segment); + assertEquals(403, statusCode); + assertEquals(BlockingContentType.AUTO, templateType); + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle index e9470075356..3c4bf51421a 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle @@ -47,6 +47,7 @@ dependencies { testImplementation project(':dd-java-agent:appsec:appsec-test-fixtures') testImplementation libs.junit.jupiter + testImplementation libs.bundles.mockito latestDepTestImplementation group: 'io.vertx', name: 'vertx-web', version: '3.+' latestDepTestImplementation group: 'io.vertx', name: 'vertx-web-client', version: '3.+' diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/gradle.lockfile b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/gradle.lockfile index b767a9cfa48..d1fae45c160 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/gradle.lockfile +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/gradle.lockfile @@ -140,7 +140,8 @@ org.junit.platform:junit-platform-runner:1.14.1=latestDepForkedTestRuntimeClassp org.junit.platform:junit-platform-suite-api:1.14.1=latestDepForkedTestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepForkedTestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=latestDepForkedTestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperTest.java new file mode 100644 index 00000000000..f1ad58e4443 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperTest.java @@ -0,0 +1,142 @@ +package datadog.trace.instrumentation.vertx_3_4.server; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.internal.TraceSegment; +import java.util.List; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Pins the blocking behavior of {@link FileUploadHelper#commitBlockingResponse}, shared by the + * multipart filenames and file-content callbacks of {@link RoutingContextFilenamesAdvice}: on a + * {@link Flow.Action.RequestBlockingAction} it commits the blocking response and returns a {@link + * BlockingException} carrying the given reason, but fails open (returns {@code null}) when there is + * no {@link BlockResponseFunction}. + * + *

vertx-web 4.0 and 5.0 have identical copies of this helper, each covered by its own test. + */ +class FileUploadHelperTest { + + private static final String FILENAMES_REASON = "Blocked request (multipart file upload)"; + private static final String CONTENT_REASON = "Blocked request (file content)"; + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private TraceSegment traceSegment; + private RequestContext reqCtx; + private Flow flow; + private RequestContext receivedCtx; + private List receivedData; + private BiFunction, Flow> cb; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUp() { + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + flow = mock(Flow.class); + cb = + (ctx, data) -> { + receivedCtx = ctx; + receivedData = data; + return flow; + }; + } + + @Test + void commitsAndReturnsExceptionOnBlockingFilenames() { + assertCommitsAndReturnsException(asList("a.txt", "b.txt"), FILENAMES_REASON); + } + + @Test + void commitsAndReturnsExceptionOnBlockingFilesContent() { + assertCommitsAndReturnsException(singletonList("file content"), CONTENT_REASON); + } + + @Test + void failsOpenWithoutBlockResponseFunction() { + List data = singletonList("a.txt"); + when(flow.getAction()).thenReturn(RBA); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertNull(FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, FILENAMES_REASON)); + + assertSame(reqCtx, receivedCtx); + assertSame(data, receivedData); + } + + @Test + void doesNotCommitWithoutBlockingAction() { + List data = singletonList("a.txt"); + when(flow.getAction()).thenReturn(Flow.Action.Noop.INSTANCE); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertNull(FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, FILENAMES_REASON)); + + assertSame(data, receivedData); + assertEquals(0, brf.calls); + } + + private void assertCommitsAndReturnsException(List data, String reason) { + when(flow.getAction()).thenReturn(RBA); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + BlockingException exception = FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, reason); + + assertNotNull(exception); + assertEquals(reason, exception.getMessage()); + assertSame(reqCtx, receivedCtx); + assertSame(data, receivedData); + brf.assertCommittedOnce(traceSegment); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return true; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceTest.java new file mode 100644 index 00000000000..c9106b50a43 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceTest.java @@ -0,0 +1,147 @@ +package datadog.trace.instrumentation.vertx_3_4.server; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import io.vertx.ext.web.Session; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Pins the blocking behavior of {@link RoutingContextSessionAdvice} by calling the advice method + * directly: on a {@link Flow.Action.RequestBlockingAction} it commits the blocking response and + * throws a {@link BlockingException}, but fails open (returns without throwing) when there is no + * {@link BlockResponseFunction}. + * + *

vertx-web 4.0 has an identical copy of this advice, covered by its own test. + */ +class RoutingContextSessionAdviceTest { + + private static final String SESSION_ID = "session-id"; + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private AgentTracer.TracerAPI originalTracer; + private TraceSegment traceSegment; + private RequestContext reqCtx; + private Session session; + private Flow flow; + private String receivedSessionId; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUp() { + originalTracer = AgentTracer.get(); + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + // the build rewrites @ActiveRequestContext to read the context from the active span, and + // skips the advice unless the context carries AppSec data + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + session = mock(Session.class); + when(session.id()).thenReturn(SESSION_ID); + flow = mock(Flow.class); + + CallbackProvider callbackProvider = mock(CallbackProvider.class); + when(callbackProvider.getCallback(EVENTS.requestSession())) + .thenReturn( + (ctx, id) -> { + receivedSessionId = id; + return flow; + }); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.activeSpan()).thenReturn(span); + when(tracer.getCallbackProvider(RequestContextSlot.APPSEC)).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @Test + void commitsAndThrowsOnBlockingAction() { + when(flow.getAction()).thenReturn(RBA); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertThrows(BlockingException.class, () -> RoutingContextSessionAdvice.after(reqCtx, session)); + + assertEquals(SESSION_ID, receivedSessionId); + brf.assertCommittedOnce(traceSegment); + } + + @Test + void failsOpenWithoutBlockResponseFunction() { + when(flow.getAction()).thenReturn(RBA); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertDoesNotThrow(() -> RoutingContextSessionAdvice.after(reqCtx, session)); + + assertEquals(SESSION_ID, receivedSessionId); + } + + @Test + void doesNotCommitWithoutBlockingAction() { + when(flow.getAction()).thenReturn(Flow.Action.Noop.INSTANCE); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertDoesNotThrow(() -> RoutingContextSessionAdvice.after(reqCtx, session)); + + assertEquals(SESSION_ID, receivedSessionId); + assertEquals(0, brf.calls); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return true; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/build.gradle b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/build.gradle index b9fdafb451f..4f27ba0d1fc 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/build.gradle +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/build.gradle @@ -48,6 +48,7 @@ dependencies { testImplementation project(':dd-java-agent:appsec:appsec-test-fixtures') testImplementation libs.junit.jupiter + testImplementation libs.bundles.mockito testRuntimeOnly project(':dd-java-agent:instrumentation:jackson-core:jackson-core-common') testRuntimeOnly project(':dd-java-agent:instrumentation:netty:netty-buffer-4.0') diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/gradle.lockfile b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/gradle.lockfile index 2576a227c29..06f9bdd23ce 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/gradle.lockfile +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/gradle.lockfile @@ -139,7 +139,8 @@ org.junit.platform:junit-platform-runner:1.14.1=latestDepTestRuntimeClasspath,te org.junit.platform:junit-platform-suite-api:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.mockito:mockito-core:4.4.0=latestDepTestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 62455354611..63d5a79345d 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -1,5 +1,6 @@ package server +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static org.junit.jupiter.api.Assumptions.assumeTrue @@ -222,6 +223,33 @@ class VertxHttpServerForkedTest extends HttpServerTest { } } } + + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching RoutingContextJsonResponseAdvice via ctx.json(). + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), '{"a": "x"}')) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } } class VertxHttpServerWorkerForkedTest extends VertxHttpServerForkedTest { diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperTest.java new file mode 100644 index 00000000000..ddf1937a08f --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperTest.java @@ -0,0 +1,142 @@ +package datadog.trace.instrumentation.vertx_4_0.server; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.internal.TraceSegment; +import java.util.List; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Pins the blocking behavior of {@link FileUploadHelper#commitBlockingResponse}, shared by the + * multipart filenames and file-content callbacks of {@link RoutingContextFilenamesAdvice}: on a + * {@link Flow.Action.RequestBlockingAction} it commits the blocking response and returns a {@link + * BlockingException} carrying the given reason, but fails open (returns {@code null}) when there is + * no {@link BlockResponseFunction}. + * + *

vertx-web 3.4 and 5.0 have identical copies of this helper, each covered by its own test. + */ +class FileUploadHelperTest { + + private static final String FILENAMES_REASON = "Blocked request (multipart file upload)"; + private static final String CONTENT_REASON = "Blocked request (file content)"; + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private TraceSegment traceSegment; + private RequestContext reqCtx; + private Flow flow; + private RequestContext receivedCtx; + private List receivedData; + private BiFunction, Flow> cb; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUp() { + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + flow = mock(Flow.class); + cb = + (ctx, data) -> { + receivedCtx = ctx; + receivedData = data; + return flow; + }; + } + + @Test + void commitsAndReturnsExceptionOnBlockingFilenames() { + assertCommitsAndReturnsException(asList("a.txt", "b.txt"), FILENAMES_REASON); + } + + @Test + void commitsAndReturnsExceptionOnBlockingFilesContent() { + assertCommitsAndReturnsException(singletonList("file content"), CONTENT_REASON); + } + + @Test + void failsOpenWithoutBlockResponseFunction() { + List data = singletonList("a.txt"); + when(flow.getAction()).thenReturn(RBA); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertNull(FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, FILENAMES_REASON)); + + assertSame(reqCtx, receivedCtx); + assertSame(data, receivedData); + } + + @Test + void doesNotCommitWithoutBlockingAction() { + List data = singletonList("a.txt"); + when(flow.getAction()).thenReturn(Flow.Action.Noop.INSTANCE); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertNull(FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, FILENAMES_REASON)); + + assertSame(data, receivedData); + assertEquals(0, brf.calls); + } + + private void assertCommitsAndReturnsException(List data, String reason) { + when(flow.getAction()).thenReturn(RBA); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + BlockingException exception = FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, reason); + + assertNotNull(exception); + assertEquals(reason, exception.getMessage()); + assertSame(reqCtx, receivedCtx); + assertSame(data, receivedData); + brf.assertCommittedOnce(traceSegment); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return true; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceTest.java new file mode 100644 index 00000000000..9df0bd98155 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceTest.java @@ -0,0 +1,147 @@ +package datadog.trace.instrumentation.vertx_4_0.server; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import io.vertx.ext.web.Session; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Pins the blocking behavior of {@link RoutingContextSessionAdvice} by calling the advice method + * directly: on a {@link Flow.Action.RequestBlockingAction} it commits the blocking response and + * throws a {@link BlockingException}, but fails open (returns without throwing) when there is no + * {@link BlockResponseFunction}. + * + *

vertx-web 3.4 has an identical copy of this advice, covered by its own test. + */ +class RoutingContextSessionAdviceTest { + + private static final String SESSION_ID = "session-id"; + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private AgentTracer.TracerAPI originalTracer; + private TraceSegment traceSegment; + private RequestContext reqCtx; + private Session session; + private Flow flow; + private String receivedSessionId; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUp() { + originalTracer = AgentTracer.get(); + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + // the build rewrites @ActiveRequestContext to read the context from the active span, and + // skips the advice unless the context carries AppSec data + when(reqCtx.getData(RequestContextSlot.APPSEC)).thenReturn(new Object()); + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + session = mock(Session.class); + when(session.id()).thenReturn(SESSION_ID); + flow = mock(Flow.class); + + CallbackProvider callbackProvider = mock(CallbackProvider.class); + when(callbackProvider.getCallback(EVENTS.requestSession())) + .thenReturn( + (ctx, id) -> { + receivedSessionId = id; + return flow; + }); + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.activeSpan()).thenReturn(span); + when(tracer.getCallbackProvider(RequestContextSlot.APPSEC)).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @Test + void commitsAndThrowsOnBlockingAction() { + when(flow.getAction()).thenReturn(RBA); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertThrows(BlockingException.class, () -> RoutingContextSessionAdvice.after(reqCtx, session)); + + assertEquals(SESSION_ID, receivedSessionId); + brf.assertCommittedOnce(traceSegment); + } + + @Test + void failsOpenWithoutBlockResponseFunction() { + when(flow.getAction()).thenReturn(RBA); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertDoesNotThrow(() -> RoutingContextSessionAdvice.after(reqCtx, session)); + + assertEquals(SESSION_ID, receivedSessionId); + } + + @Test + void doesNotCommitWithoutBlockingAction() { + when(flow.getAction()).thenReturn(Flow.Action.Noop.INSTANCE); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertDoesNotThrow(() -> RoutingContextSessionAdvice.after(reqCtx, session)); + + assertEquals(SESSION_ID, receivedSessionId); + assertEquals(0, brf.calls); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return true; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/build.gradle b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/build.gradle index 443976e9750..9bfec1533ad 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/build.gradle +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/build.gradle @@ -51,6 +51,8 @@ dependencies { testImplementation project(':dd-java-agent:appsec:appsec-test-fixtures') + testImplementation libs.bundles.mockito + testRuntimeOnly project(':dd-java-agent:instrumentation:jackson-core:jackson-core-common') testRuntimeOnly project(':dd-java-agent:instrumentation:netty:netty-buffer-4.0') diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/gradle.lockfile b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/gradle.lockfile index ba2d485530e..3b0e6cfebad 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/gradle.lockfile +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/gradle.lockfile @@ -153,7 +153,8 @@ org.junit.platform:junit-platform-suite-api:1.14.1=latestDepForkedTestRuntimeCla org.junit.platform:junit-platform-suite-commons:1.14.1=latestDepForkedTestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit:junit-bom:5.14.1=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.junit:junit-bom:6.1.2=spotbugs -org.mockito:mockito-core:4.4.0=latestDepForkedTestRuntimeClasspath,latestDepTestRuntimeClasspath,testRuntimeClasspath +org.mockito:mockito-core:4.4.0=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:4.4.0=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.objenesis:objenesis:3.3=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.opentest4j:opentest4j:1.3.0=latestDepForkedTestCompileClasspath,latestDepForkedTestRuntimeClasspath,latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.ow2.asm:asm-analysis:9.10.1=spotbugs diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index cdd4bca1c19..3fd805532a0 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -217,6 +217,33 @@ class VertxHttpServerForkedTest extends HttpServerTest { } } } + + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching RoutingContextJsonResponseAdvice via ctx.json(). + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), '{"a": "x"}')) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } } class VertxHttpServerWorkerForkedTest extends VertxHttpServerForkedTest { diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperTest.java new file mode 100644 index 00000000000..43e3c2e3a3a --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperTest.java @@ -0,0 +1,142 @@ +package datadog.trace.instrumentation.vertx_5_0.server; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.internal.TraceSegment; +import java.util.List; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Pins the blocking behavior of {@link FileUploadHelper#commitBlockingResponse}, shared by the + * multipart filenames and file-content callbacks of {@link RoutingContextFilenamesAdvice}: on a + * {@link Flow.Action.RequestBlockingAction} it commits the blocking response and returns a {@link + * BlockingException} carrying the given reason, but fails open (returns {@code null}) when there is + * no {@link BlockResponseFunction}. + * + *

vertx-web 3.4 and 4.0 have identical copies of this helper, each covered by its own test. + */ +class FileUploadHelperTest { + + private static final String FILENAMES_REASON = "Blocked request (multipart file upload)"; + private static final String CONTENT_REASON = "Blocked request (file content)"; + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private TraceSegment traceSegment; + private RequestContext reqCtx; + private Flow flow; + private RequestContext receivedCtx; + private List receivedData; + private BiFunction, Flow> cb; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUp() { + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + flow = mock(Flow.class); + cb = + (ctx, data) -> { + receivedCtx = ctx; + receivedData = data; + return flow; + }; + } + + @Test + void commitsAndReturnsExceptionOnBlockingFilenames() { + assertCommitsAndReturnsException(asList("a.txt", "b.txt"), FILENAMES_REASON); + } + + @Test + void commitsAndReturnsExceptionOnBlockingFilesContent() { + assertCommitsAndReturnsException(singletonList("file content"), CONTENT_REASON); + } + + @Test + void failsOpenWithoutBlockResponseFunction() { + List data = singletonList("a.txt"); + when(flow.getAction()).thenReturn(RBA); + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertNull(FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, FILENAMES_REASON)); + + assertSame(reqCtx, receivedCtx); + assertSame(data, receivedData); + } + + @Test + void doesNotCommitWithoutBlockingAction() { + List data = singletonList("a.txt"); + when(flow.getAction()).thenReturn(Flow.Action.Noop.INSTANCE); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertNull(FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, FILENAMES_REASON)); + + assertSame(data, receivedData); + assertEquals(0, brf.calls); + } + + private void assertCommitsAndReturnsException(List data, String reason) { + when(flow.getAction()).thenReturn(RBA); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + BlockingException exception = FileUploadHelper.commitBlockingResponse(cb, reqCtx, data, reason); + + assertNotNull(exception); + assertEquals(reason, exception.getMessage()); + assertSame(reqCtx, receivedCtx); + assertSame(data, receivedData); + brf.assertCommittedOnce(traceSegment); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return true; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/internal-api/src/test/groovy/datadog/trace/api/http/StoredCharBodyTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/http/StoredCharBodyTest.groovy deleted file mode 100644 index 18696fa273d..00000000000 --- a/internal-api/src/test/groovy/datadog/trace/api/http/StoredCharBodyTest.groovy +++ /dev/null @@ -1,77 +0,0 @@ -package datadog.trace.api.http - -import datadog.trace.api.gateway.Flow -import datadog.trace.api.gateway.RequestContext -import spock.lang.Specification - -import java.util.function.BiFunction - -class StoredCharBodyTest extends Specification { - RequestContext requestContext = Mock(RequestContext) { - _ * getData(_) >> it - } - BiFunction startCb = Mock() - BiFunction > endCb = Mock() - - StoredCharBody storedCharBody = new StoredCharBody(requestContext, startCb, endCb, 1) - - void 'basic test with no buffer extension'() { - Flow flow = Mock() - - when: - storedCharBody.appendData('a') - - then: - 1 * startCb.apply(requestContext, storedCharBody) - - when: - storedCharBody.appendData((int) 'a') - storedCharBody.appendData(['a' as char]* 126 as char[], 0, 126) - def resFlow = storedCharBody.maybeNotify() - - then: - 1 * endCb.apply(requestContext, storedCharBody) >> flow - storedCharBody.get().toString() == 'a' * 128 - resFlow.is(flow) - } - - void 'has a cutoff at 128k chars'() { - when: - storedCharBody.appendData('a') - storedCharBody.appendData('a' * (128 * 1024)) // last ignored - storedCharBody.appendData((int) 'a') // ignored - storedCharBody.appendData('a') // ignored - storedCharBody.appendData(['a' as char] as char[], 0, 1) // ignored - - then: - 1 * startCb.apply(requestContext, storedCharBody) - } - - void 'insert invalid data'() { - when: - storedCharBody.appendData(-1) - - then: - storedCharBody.get() as String == '' - } - - void 'insert empty range'() { - when: - storedCharBody.appendData([] as char[], 0, 0) - - then: - storedCharBody.get() as String == '' - } - - void 'exercise maybeNotify and get on empty object'() { - when: - storedCharBody.maybeNotify() - - then: - 1 * startCb.apply(requestContext, storedCharBody) - then: - 1 * endCb.apply(requestContext, storedCharBody) - then: - storedCharBody.get() as String == '' - } -} diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java index e07f49ceb12..a850e5b0b52 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/BlockResponseFunctionTest.java @@ -1,7 +1,10 @@ package datadog.trace.api.gateway; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -9,20 +12,72 @@ import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.internal.TraceSegment; import datadog.trace.bootstrap.instrumentation.api.ClientIpAddressData; +import java.util.HashMap; import java.util.Map; import java.util.function.Function; import org.junit.jupiter.api.Test; /** - * Covers the {@code tryCommitBlockingResponse(RequestContext, RequestBlockingAction)} default - * method, which reports a block failure to {@link AppSecContext#reportBlockFailure()} when the - * blocking response cannot be committed. + * Covers the {@link BlockResponseFunction} default methods: {@code + * tryCommitBlockingResponse(TraceSegment, RequestBlockingAction)}, which forwards every field of + * the action to the abstract parameter-based method, and {@code + * tryCommitBlockingResponse(RequestContext, RequestBlockingAction)}, which reports a block failure + * to {@link AppSecContext#reportBlockFailure()} when the blocking response cannot be committed. */ class BlockResponseFunctionTest { private static final Flow.Action.RequestBlockingAction RBA = new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + @Test + void segmentOverloadForwardsAllActionFieldsToParameterBasedMethod() { + Map extraHeaders = new HashMap<>(); + extraHeaders.put("X-Custom", "custom-value"); + extraHeaders.put("Location", "https://example.com/blocked"); + Flow.Action.RequestBlockingAction rba = + new Flow.Action.RequestBlockingAction( + 418, BlockingContentType.HTML, extraHeaders, "security-response-id"); + TraceSegment segment = TraceSegment.NoOp.INSTANCE; + TestBlockResponseFunction brf = new TestBlockResponseFunction(true); + + assertTrue(brf.tryCommitBlockingResponse(segment, rba)); + + assertEquals(1, brf.invocations); + assertSame(segment, brf.lastSegment); + assertEquals(418, brf.lastStatusCode); + assertEquals(BlockingContentType.HTML, brf.lastTemplateType); + assertSame(extraHeaders, brf.lastExtraHeaders); + assertEquals("security-response-id", brf.lastSecurityResponseId); + } + + @Test + void segmentOverloadForwardsRedirectActionFields() { + Flow.Action.RequestBlockingAction rba = + Flow.Action.RequestBlockingAction.forRedirect( + 302, "https://example.com/redirect", "redirect-response-id"); + TestBlockResponseFunction brf = new TestBlockResponseFunction(true); + + assertTrue(brf.tryCommitBlockingResponse(TraceSegment.NoOp.INSTANCE, rba)); + + assertEquals(302, brf.lastStatusCode); + assertEquals(BlockingContentType.NONE, brf.lastTemplateType); + assertEquals(singletonMap("Location", "https://example.com/redirect"), brf.lastExtraHeaders); + assertEquals("redirect-response-id", brf.lastSecurityResponseId); + } + + @Test + void segmentOverloadForwardsDefaultsAndPropagatesFailedCommit() { + TestBlockResponseFunction brf = new TestBlockResponseFunction(false); + + assertFalse(brf.tryCommitBlockingResponse(TraceSegment.NoOp.INSTANCE, RBA)); + + assertEquals(1, brf.invocations); + assertEquals(403, brf.lastStatusCode); + assertEquals(BlockingContentType.AUTO, brf.lastTemplateType); + assertEquals(emptyMap(), brf.lastExtraHeaders); + assertNull(brf.lastSecurityResponseId); + } + @Test void doesNotReportBlockFailureWhenCommitSucceeds() { CountingAppSecContext appSecCtx = new CountingAppSecContext(); @@ -76,6 +131,9 @@ private static final class TestBlockResponseFunction implements BlockResponseFun private TraceSegment lastSegment; private int lastStatusCode; private BlockingContentType lastTemplateType; + private Map lastExtraHeaders; + private String lastSecurityResponseId; + private int invocations; private TestBlockResponseFunction(boolean committed) { this.committed = committed; @@ -91,6 +149,9 @@ public boolean tryCommitBlockingResponse( this.lastSegment = segment; this.lastStatusCode = statusCode; this.lastTemplateType = templateType; + this.lastExtraHeaders = extraHeaders; + this.lastSecurityResponseId = securityResponseId; + this.invocations++; return committed; } } diff --git a/internal-api/src/test/java/datadog/trace/api/http/StoredCharBodyTest.java b/internal-api/src/test/java/datadog/trace/api/http/StoredCharBodyTest.java new file mode 100644 index 00000000000..fc8e627e50c --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/http/StoredCharBodyTest.java @@ -0,0 +1,159 @@ +package datadog.trace.api.http; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.internal.TraceSegment; +import java.util.Arrays; +import java.util.Map; +import java.util.function.BiFunction; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.InOrder; + +class StoredCharBodyTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private RequestContext requestContext; + private BiFunction startCb; + private BiFunction> endCb; + private StoredCharBody storedCharBody; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUp() { + requestContext = mock(RequestContext.class); + startCb = mock(BiFunction.class); + endCb = mock(BiFunction.class); + storedCharBody = new StoredCharBody(requestContext, startCb, endCb, 1); + } + + @Test + void basicTestWithNoBufferExtension() { + @SuppressWarnings("unchecked") + Flow flow = mock(Flow.class); + + storedCharBody.appendData("a"); + + verify(startCb).apply(requestContext, storedCharBody); + + when(endCb.apply(requestContext, storedCharBody)).thenReturn(flow); + storedCharBody.appendData((int) 'a'); + storedCharBody.appendData(repeat('a', 126), 0, 126); + Flow resFlow = storedCharBody.maybeNotify(); + + verify(endCb).apply(requestContext, storedCharBody); + assertEquals(new String(repeat('a', 128)), storedCharBody.get().toString()); + assertSame(flow, resFlow); + } + + @Test + void hasACutoffAt128kChars() { + storedCharBody.appendData("a"); + storedCharBody.appendData(new String(repeat('a', 128 * 1024))); // last ignored + storedCharBody.appendData((int) 'a'); // ignored + storedCharBody.appendData("a"); // ignored + storedCharBody.appendData(new char[] {'a'}, 0, 1); // ignored + + verify(startCb).apply(requestContext, storedCharBody); + } + + @Test + void insertInvalidData() { + storedCharBody.appendData(-1); + + assertEquals("", storedCharBody.get().toString()); + } + + @Test + void insertEmptyRange() { + storedCharBody.appendData(new char[0], 0, 0); + + assertEquals("", storedCharBody.get().toString()); + } + + @Test + void exerciseMaybeNotifyAndGetOnEmptyObject() { + storedCharBody.maybeNotify(); + + InOrder ordered = inOrder(startCb, endCb); + ordered.verify(startCb).apply(requestContext, storedCharBody); + ordered.verify(endCb).apply(requestContext, storedCharBody); + assertEquals("", storedCharBody.get().toString()); + } + + @ParameterizedTest(name = "commit succeeds: {0}") + @ValueSource(booleans = {true, false}) + void maybeNotifyAndBlockCommitsBlockingResponseAndThrows(boolean commitResult) { + TraceSegment traceSegment = mock(TraceSegment.class); + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(requestContext.getTraceSegment()).thenReturn(traceSegment); + when(requestContext.getBlockResponseFunction()).thenReturn(brf); + @SuppressWarnings("unchecked") + Flow blockingFlow = mock(Flow.class); + when(blockingFlow.getAction()).thenReturn(RBA); + when(endCb.apply(requestContext, storedCharBody)).thenReturn(blockingFlow); + + storedCharBody.appendData("a"); + + assertThrows(BlockingException.class, storedCharBody::maybeNotifyAndBlock); + brf.assertCommittedOnce(traceSegment); + } + + private static char[] repeat(char value, int count) { + char[] chars = new char[count]; + Arrays.fill(chars, value); + return chars; + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + private RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return commitResult; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpersBlockingTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpersBlockingTest.java new file mode 100644 index 00000000000..c5c3265d883 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpersBlockingTest.java @@ -0,0 +1,153 @@ +package datadog.trace.bootstrap.instrumentation.api.java.lang; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.internal.TraceSegment; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Pins the fail-closed blocking behavior of the CMDI and SHI RASP checks: when the AppSec callback + * returns a real {@link Flow.Action.RequestBlockingAction}, a {@link BlockingException} is always + * thrown, whether or not a {@link BlockResponseFunction} is present and regardless of whether it + * managed to commit the blocking response. + */ +class ProcessImplInstrumentationHelpersBlockingTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON); + + private static final String[] CMD_ARRAY = {"/bin/../usr/bin/reboot", "-f"}; + private static final String SHELL_CMD = "/bin/../usr/bin/reboot -f"; + + private AgentTracer.TracerAPI originalTracer; + private TraceSegment traceSegment; + private RequestContext reqCtx; + + @BeforeEach + void setUp() { + // shiRaspCheck sets a ThreadLocal guard that makes cmdiRaspCheck a no-op until reset + ProcessImplInstrumentationHelpers.resetCheckShi(); + + originalTracer = AgentTracer.get(); + traceSegment = mock(TraceSegment.class); + reqCtx = mock(RequestContext.class); + when(reqCtx.getTraceSegment()).thenReturn(traceSegment); + + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(reqCtx); + + @SuppressWarnings("unchecked") + Flow blockingFlow = mock(Flow.class); + when(blockingFlow.getAction()).thenReturn(RBA); + CallbackProvider callbackProvider = mock(CallbackProvider.class); + when(callbackProvider.getCallback(EVENTS.execCmd())).thenReturn((ctx, cmd) -> blockingFlow); + when(callbackProvider.getCallback(EVENTS.shellCmd())).thenReturn((ctx, cmd) -> blockingFlow); + + AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.activeSpan()).thenReturn(span); + when(tracer.getCallbackProvider(RequestContextSlot.APPSEC)).thenReturn(callbackProvider); + AgentTracer.forceRegister(tracer); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + ProcessImplInstrumentationHelpers.resetCheckShi(); + } + + @Test + void cmdiThrowsWithoutBlockResponseFunction() { + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertThrows( + BlockingException.class, () -> ProcessImplInstrumentationHelpers.cmdiRaspCheck(CMD_ARRAY)); + } + + @ParameterizedTest(name = "commit succeeds: {0}") + @ValueSource(booleans = {true, false}) + void cmdiThrowsWithBlockResponseFunction(boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertThrows( + BlockingException.class, () -> ProcessImplInstrumentationHelpers.cmdiRaspCheck(CMD_ARRAY)); + + brf.assertCommittedOnce(traceSegment); + } + + @Test + void shiThrowsWithoutBlockResponseFunction() { + when(reqCtx.getBlockResponseFunction()).thenReturn(null); + + assertThrows( + BlockingException.class, () -> ProcessImplInstrumentationHelpers.shiRaspCheck(SHELL_CMD)); + } + + @ParameterizedTest(name = "commit succeeds: {0}") + @ValueSource(booleans = {true, false}) + void shiThrowsWithBlockResponseFunction(boolean commitResult) { + RecordingBlockResponseFunction brf = new RecordingBlockResponseFunction(commitResult); + when(reqCtx.getBlockResponseFunction()).thenReturn(brf); + + assertThrows( + BlockingException.class, () -> ProcessImplInstrumentationHelpers.shiRaspCheck(SHELL_CMD)); + + brf.assertCommittedOnce(traceSegment); + } + + /** + * Hand-written fake that only implements the abstract 5-arg method, so it records the commit + * whichever {@code tryCommitBlockingResponse} overload the production code calls. + */ + private static final class RecordingBlockResponseFunction implements BlockResponseFunction { + private final boolean commitResult; + private int calls; + private TraceSegment lastSegment; + private int lastStatusCode; + private BlockingContentType lastTemplateType; + + private RecordingBlockResponseFunction(boolean commitResult) { + this.commitResult = commitResult; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + calls++; + lastSegment = segment; + lastStatusCode = statusCode; + lastTemplateType = templateType; + return commitResult; + } + + private void assertCommittedOnce(TraceSegment expectedSegment) { + assertEquals(1, calls); + assertSame(expectedSegment, lastSegment); + assertEquals(RBA.getStatusCode(), lastStatusCode); + assertEquals(RBA.getBlockingContentType(), lastTemplateType); + } + } +} From 55609512781caf5ba08ed6de33106bf38b6980a4 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 24 Sep 2026 15:50:34 +0200 Subject: [PATCH 2/6] test: rename AppSec-blocking test classes to match *AppSec* CODEOWNERS pattern - RoutingContextSessionAdviceTest -> RoutingContextSessionAdviceAppSecTest (vertx-web 3.4, 4.0) - FileUploadHelperTest -> FileUploadHelperAppSecTest (vertx-web 3.4, 4.0, 5.0) - MessageBodyWriterAdviceTest -> MessageBodyWriterAdviceAppSecTest (jakarta-rs-annotations-3.0, jax-rs-annotations-2.0) --- ...erAdviceTest.java => MessageBodyWriterAdviceAppSecTest.java} | 2 +- ...erAdviceTest.java => MessageBodyWriterAdviceAppSecTest.java} | 2 +- ...ileUploadHelperTest.java => FileUploadHelperAppSecTest.java} | 2 +- ...viceTest.java => RoutingContextSessionAdviceAppSecTest.java} | 2 +- ...ileUploadHelperTest.java => FileUploadHelperAppSecTest.java} | 2 +- ...viceTest.java => RoutingContextSessionAdviceAppSecTest.java} | 2 +- ...ileUploadHelperTest.java => FileUploadHelperAppSecTest.java} | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) rename dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/{MessageBodyWriterAdviceTest.java => MessageBodyWriterAdviceAppSecTest.java} (99%) rename dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/{MessageBodyWriterAdviceTest.java => MessageBodyWriterAdviceAppSecTest.java} (99%) rename dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/{FileUploadHelperTest.java => FileUploadHelperAppSecTest.java} (99%) rename dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/{RoutingContextSessionAdviceTest.java => RoutingContextSessionAdviceAppSecTest.java} (99%) rename dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/{FileUploadHelperTest.java => FileUploadHelperAppSecTest.java} (99%) rename dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/{RoutingContextSessionAdviceTest.java => RoutingContextSessionAdviceAppSecTest.java} (99%) rename dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/{FileUploadHelperTest.java => FileUploadHelperAppSecTest.java} (99%) diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceTest.java b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceAppSecTest.java similarity index 99% rename from dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceTest.java rename to dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceAppSecTest.java index ebec1eb2da2..52f36f81a8e 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceTest.java +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/java/datadog/trace/instrumentation/jakarta3/MessageBodyWriterAdviceAppSecTest.java @@ -35,7 +35,7 @@ * BlockingException} regardless of the commit result. Non-JSON media types never reach the * callback. */ -class MessageBodyWriterAdviceTest { +class MessageBodyWriterAdviceAppSecTest { private static final Flow.Action.RequestBlockingAction RBA = new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceTest.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceAppSecTest.java similarity index 99% rename from dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceTest.java rename to dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceAppSecTest.java index 278b38f2f17..4529ab1ada7 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceTest.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/java/datadog/trace/instrumentation/jaxrs2/MessageBodyWriterAdviceAppSecTest.java @@ -35,7 +35,7 @@ * BlockingException} regardless of the commit result. Non-JSON media types never reach the * callback. */ -class MessageBodyWriterAdviceTest { +class MessageBodyWriterAdviceAppSecTest { private static final Flow.Action.RequestBlockingAction RBA = new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperAppSecTest.java similarity index 99% rename from dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperTest.java rename to dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperAppSecTest.java index f1ad58e4443..b29621d4da9 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperTest.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelperAppSecTest.java @@ -30,7 +30,7 @@ * *

vertx-web 4.0 and 5.0 have identical copies of this helper, each covered by its own test. */ -class FileUploadHelperTest { +class FileUploadHelperAppSecTest { private static final String FILENAMES_REASON = "Blocked request (multipart file upload)"; private static final String CONTENT_REASON = "Blocked request (file content)"; diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceAppSecTest.java similarity index 99% rename from dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceTest.java rename to dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceAppSecTest.java index c9106b50a43..da4d48797d1 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceTest.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextSessionAdviceAppSecTest.java @@ -32,7 +32,7 @@ * *

vertx-web 4.0 has an identical copy of this advice, covered by its own test. */ -class RoutingContextSessionAdviceTest { +class RoutingContextSessionAdviceAppSecTest { private static final String SESSION_ID = "session-id"; private static final Flow.Action.RequestBlockingAction RBA = diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperAppSecTest.java similarity index 99% rename from dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperTest.java rename to dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperAppSecTest.java index ddf1937a08f..8238d804747 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperTest.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelperAppSecTest.java @@ -30,7 +30,7 @@ * *

vertx-web 3.4 and 5.0 have identical copies of this helper, each covered by its own test. */ -class FileUploadHelperTest { +class FileUploadHelperAppSecTest { private static final String FILENAMES_REASON = "Blocked request (multipart file upload)"; private static final String CONTENT_REASON = "Blocked request (file content)"; diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceAppSecTest.java similarity index 99% rename from dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceTest.java rename to dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceAppSecTest.java index 9df0bd98155..114b8369dba 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceTest.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextSessionAdviceAppSecTest.java @@ -32,7 +32,7 @@ * *

vertx-web 3.4 has an identical copy of this advice, covered by its own test. */ -class RoutingContextSessionAdviceTest { +class RoutingContextSessionAdviceAppSecTest { private static final String SESSION_ID = "session-id"; private static final Flow.Action.RequestBlockingAction RBA = diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperAppSecTest.java similarity index 99% rename from dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperTest.java rename to dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperAppSecTest.java index 43e3c2e3a3a..68299007265 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperTest.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelperAppSecTest.java @@ -30,7 +30,7 @@ * *

vertx-web 3.4 and 4.0 have identical copies of this helper, each covered by its own test. */ -class FileUploadHelperTest { +class FileUploadHelperAppSecTest { private static final String FILENAMES_REASON = "Blocked request (multipart file upload)"; private static final String CONTENT_REASON = "Blocked request (file content)"; From b159e934c6c525bb6c784e5be0f70d17c2ddaaba Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 08:13:44 +0200 Subject: [PATCH 3/6] test: split AppSec blocking tests out of shared HttpServerTest suites Move the "test blocking on json response body" feature out of RatpackHttpServerTest, VertxHttpServerForkedTest (4.0, 5.0) and PlayServerTest (2.5, 2.6) into new *AppSec* test classes, so they match the CODEOWNERS *AppSec* pattern without editing CODEOWNERS or renaming the shared, general-purpose contract-test files. The new classes extend the original test class to reuse its server bootstrap and helpers, and are annotated with the new @ExcludeInheritedFeatures Spock extension (utils/test-utils) so the ~22 inherited generic contract tests are excluded at discovery time instead of re-running. --- .../play25/server/PlayServerAppSecTest.groovy | 67 +++++++++++++++++++ .../play25/server/PlayServerTest.groovy | 36 ---------- .../play26/server/PlayServerAppSecTest.groovy | 42 ++++++++++++ .../play26/server/PlayServerTest.groovy | 32 --------- .../server/RatpackHttpServerAppSecTest.groovy | 42 ++++++++++++ .../server/RatpackHttpServerTest.groovy | 34 ---------- .../VertxHttpServerAppSecForkedTest.groovy | 46 +++++++++++++ .../server/VertxHttpServerForkedTest.groovy | 28 -------- .../VertxHttpServerAppSecForkedTest.groovy | 46 +++++++++++++ .../server/VertxHttpServerForkedTest.groovy | 27 -------- utils/test-utils/build.gradle.kts | 1 + .../test/util/ExcludeInheritedFeatures.java | 18 +++++ .../ExcludeInheritedFeaturesExtension.java | 21 ++++++ 13 files changed, 283 insertions(+), 157 deletions(-) create mode 100644 dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy create mode 100644 dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy create mode 100644 dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerAppSecTest.groovy create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy create mode 100644 utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeatures.java create mode 100644 utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeaturesExtension.java diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy new file mode 100644 index 00000000000..c2d38efe9c4 --- /dev/null +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy @@ -0,0 +1,67 @@ +package datadog.trace.instrumentation.play25.server + +import datadog.trace.agent.test.base.HttpServer +import datadog.trace.instrumentation.play25.PlayRoutersScala +import datadog.trace.test.util.ExcludeInheritedFeatures +import groovy.json.JsonOutput +import groovy.transform.CompileStatic +import okhttp3.MediaType +import okhttp3.RequestBody +import spock.lang.Shared + +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON +import static org.junit.jupiter.api.Assumptions.assumeTrue + +@ExcludeInheritedFeatures +class PlayServerAppSecTest extends PlayServerTest { + + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching StatusHeaderSendJsonAdvice (Java routers) or ResultsStatusApplyAdvice (Scala routers). + */ + def 'test blocking on json response body'() { + setup: + assumeTrue(testBlockingOnResponse() && testResponseBodyJson()) + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } +} + +class PlayScalaAsyncServerAppSecTest extends PlayServerAppSecTest { + @Shared + ExecutorService executor + + def cleanupSpec() { + executor.shutdown() + } + + @Override + @CompileStatic + HttpServer server() { + executor = Executors.newCachedThreadPool() + new PlayHttpServer(PlayRoutersScala.async(executor).asJava()) + } +} diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy index 5f9465c913f..fd10be014f3 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerTest.groovy @@ -7,18 +7,13 @@ import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.play24.PlayHttpServerDecorator -import groovy.json.JsonOutput import groovy.transform.CompileStatic -import okhttp3.MediaType -import okhttp3.RequestBody import play.server.Server -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS -import static org.junit.jupiter.api.Assumptions.assumeTrue class PlayServerTest extends HttpServerTest { @@ -113,37 +108,6 @@ class PlayServerTest extends HttpServerTest { true } - /** - * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), - * reaching StatusHeaderSendJsonAdvice (Java routers) or ResultsStatusApplyAdvice (Scala routers). - */ - def 'test blocking on json response body'() { - setup: - assumeTrue(testBlockingOnResponse() && testResponseBodyJson()) - def request = request( - BODY_JSON, 'POST', - RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) - .header(IG_BLOCK_RESPONSE_HEADER, 'body') - .build() - - when: - def response = client.newCall(request).execute() - - then: - if (isDataStreamsEnabled()) { - TEST_DATA_STREAMS_WRITER.waitForGroups(1) - } - response.code() == 413 - response.body().charStream().text.contains('"title":"You\'ve been blocked"') - TEST_WRITER.waitForTraces(1) - def rootSpan = TEST_WRITER.get(0).find { - it.parentId == 0 - } - rootSpan != null - rootSpan.tags['http.status_code'] == 413 - rootSpan.tags['appsec.blocked'] == 'true' - } - @Override String testPathParam() { '/path/?/param' diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy new file mode 100644 index 00000000000..94f20208a6e --- /dev/null +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy @@ -0,0 +1,42 @@ +package datadog.trace.instrumentation.play26.server + +import datadog.trace.test.util.ExcludeInheritedFeatures +import groovy.json.JsonOutput +import okhttp3.MediaType +import okhttp3.RequestBody + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON + +@ExcludeInheritedFeatures +class PlayServerAppSecTest extends PlayServerTest { + + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching StatusHeaderSendJsonAdvice. + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } +} diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy index e889a3122ad..0ac729a38ac 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy @@ -1,10 +1,8 @@ package datadog.trace.instrumentation.play26.server -import groovy.json.JsonOutput import okhttp3.MediaType import okhttp3.RequestBody -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_XML class PlayServerTest extends AbstractPlayServerTest { @@ -19,36 +17,6 @@ class PlayServerTest extends AbstractPlayServerTest { true } - /** - * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), - * reaching StatusHeaderSendJsonAdvice. - */ - def 'test blocking on json response body'() { - setup: - def request = request( - BODY_JSON, 'POST', - RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) - .header(IG_BLOCK_RESPONSE_HEADER, 'body') - .build() - - when: - def response = client.newCall(request).execute() - - then: - if (isDataStreamsEnabled()) { - TEST_DATA_STREAMS_WRITER.waitForGroups(1) - } - response.code() == 413 - response.body().charStream().text.contains('"title":"You\'ve been blocked"') - TEST_WRITER.waitForTraces(1) - def rootSpan = TEST_WRITER.get(0).find { - it.parentId == 0 - } - rootSpan != null - rootSpan.tags['http.status_code'] == 413 - rootSpan.tags['appsec.blocked'] == 'true' - } - def 'test instrumentation gateway xml request body'() { setup: def request = request( diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerAppSecTest.groovy b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerAppSecTest.groovy new file mode 100644 index 00000000000..2654bfb8b82 --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerAppSecTest.groovy @@ -0,0 +1,42 @@ +package server + +import datadog.trace.test.util.ExcludeInheritedFeatures +import groovy.json.JsonOutput +import okhttp3.MediaType +import okhttp3.RequestBody + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON + +@ExcludeInheritedFeatures +class RatpackHttpServerAppSecTest extends RatpackHttpServerTest { + + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching JsonRendererAdvice. + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy index 80dd8f9d59b..0b665683b2b 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy @@ -8,12 +8,8 @@ import datadog.trace.api.DDTags import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator import datadog.trace.instrumentation.ratpack.RatpackServerDecorator -import groovy.json.JsonOutput -import okhttp3.MediaType -import okhttp3.RequestBody import ratpack.test.embed.EmbeddedApp -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED @@ -128,36 +124,6 @@ class RatpackHttpServerTest extends HttpServerTest { true } - /** - * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), - * reaching JsonRendererAdvice. - */ - def 'test blocking on json response body'() { - setup: - def request = request( - BODY_JSON, 'POST', - RequestBody.create(MediaType.get('application/json'), JsonOutput.toJson([a: 'x']))) - .header(IG_BLOCK_RESPONSE_HEADER, 'body') - .build() - - when: - def response = client.newCall(request).execute() - - then: - if (isDataStreamsEnabled()) { - TEST_DATA_STREAMS_WRITER.waitForGroups(1) - } - response.code() == 413 - response.body().charStream().text.contains('"title":"You\'ve been blocked"') - TEST_WRITER.waitForTraces(1) - def rootSpan = TEST_WRITER.get(0).find { - it.parentId == 0 - } - rootSpan != null - rootSpan.tags['http.status_code'] == 413 - rootSpan.tags['appsec.blocked'] == 'true' - } - @Override void handlerSpan(TraceAssert trace, ServerEndpoint endpoint = SUCCESS) { trace.span { diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy new file mode 100644 index 00000000000..f3f1b878592 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -0,0 +1,46 @@ +package server + +import datadog.trace.agent.test.base.HttpServer +import datadog.trace.test.util.ExcludeInheritedFeatures +import okhttp3.MediaType +import okhttp3.RequestBody + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON + +@ExcludeInheritedFeatures +class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { + + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching RoutingContextJsonResponseAdvice via ctx.json(). + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), '{"a": "x"}')) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } +} + +class VertxHttpServerWorkerAppSecForkedTest extends VertxHttpServerAppSecForkedTest { + @Override + HttpServer server() { + return new VertxServer(verticle(), routerBasePath(), true) + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 63d5a79345d..62455354611 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -1,6 +1,5 @@ package server -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static org.junit.jupiter.api.Assumptions.assumeTrue @@ -223,33 +222,6 @@ class VertxHttpServerForkedTest extends HttpServerTest { } } } - - /** - * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), - * reaching RoutingContextJsonResponseAdvice via ctx.json(). - */ - def 'test blocking on json response body'() { - setup: - def request = request( - BODY_JSON, 'POST', - RequestBody.create(MediaType.get('application/json'), '{"a": "x"}')) - .header(IG_BLOCK_RESPONSE_HEADER, 'body') - .build() - - when: - def response = client.newCall(request).execute() - - then: - response.code() == 413 - response.body().charStream().text.contains('"title":"You\'ve been blocked"') - TEST_WRITER.waitForTraces(1) - def rootSpan = TEST_WRITER.get(0).find { - it.parentId == 0 - } - rootSpan != null - rootSpan.tags['http.status_code'] == 413 - rootSpan.tags['appsec.blocked'] == 'true' - } } class VertxHttpServerWorkerForkedTest extends VertxHttpServerForkedTest { diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy new file mode 100644 index 00000000000..f3f1b878592 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -0,0 +1,46 @@ +package server + +import datadog.trace.agent.test.base.HttpServer +import datadog.trace.test.util.ExcludeInheritedFeatures +import okhttp3.MediaType +import okhttp3.RequestBody + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON + +@ExcludeInheritedFeatures +class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { + + /** + * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), + * reaching RoutingContextJsonResponseAdvice via ctx.json(). + */ + def 'test blocking on json response body'() { + setup: + def request = request( + BODY_JSON, 'POST', + RequestBody.create(MediaType.get('application/json'), '{"a": "x"}')) + .header(IG_BLOCK_RESPONSE_HEADER, 'body') + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + def rootSpan = TEST_WRITER.get(0).find { + it.parentId == 0 + } + rootSpan != null + rootSpan.tags['http.status_code'] == 413 + rootSpan.tags['appsec.blocked'] == 'true' + } +} + +class VertxHttpServerWorkerAppSecForkedTest extends VertxHttpServerAppSecForkedTest { + @Override + HttpServer server() { + return new VertxServer(verticle(), routerBasePath(), true) + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 3fd805532a0..cdd4bca1c19 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -217,33 +217,6 @@ class VertxHttpServerForkedTest extends HttpServerTest { } } } - - /** - * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), - * reaching RoutingContextJsonResponseAdvice via ctx.json(). - */ - def 'test blocking on json response body'() { - setup: - def request = request( - BODY_JSON, 'POST', - RequestBody.create(MediaType.get('application/json'), '{"a": "x"}')) - .header(IG_BLOCK_RESPONSE_HEADER, 'body') - .build() - - when: - def response = client.newCall(request).execute() - - then: - response.code() == 413 - response.body().charStream().text.contains('"title":"You\'ve been blocked"') - TEST_WRITER.waitForTraces(1) - def rootSpan = TEST_WRITER.get(0).find { - it.parentId == 0 - } - rootSpan != null - rootSpan.tags['http.status_code'] == 413 - rootSpan.tags['appsec.blocked'] == 'true' - } } class VertxHttpServerWorkerForkedTest extends VertxHttpServerForkedTest { diff --git a/utils/test-utils/build.gradle.kts b/utils/test-utils/build.gradle.kts index c47732851d6..a2c4003063a 100644 --- a/utils/test-utils/build.gradle.kts +++ b/utils/test-utils/build.gradle.kts @@ -18,6 +18,7 @@ extra["excludedClassesCoverage"] = listOf( "datadog.trace.test.util.ConfigTransformSpockExtension*", "datadog.trace.test.util.ControllableEnvironmentVariables*", "datadog.trace.test.util.DDSpecification*", + "datadog.trace.test.util.ExcludeInheritedFeatures*", "datadog.trace.test.util.Flaky*", "datadog.trace.test.util.FlakySpockExtension*", "datadog.trace.test.util.MultipartRequestParser*", diff --git a/utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeatures.java b/utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeatures.java new file mode 100644 index 00000000000..8c15a30b882 --- /dev/null +++ b/utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeatures.java @@ -0,0 +1,18 @@ +package datadog.trace.test.util; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.spockframework.runtime.extension.ExtensionAnnotation; + +/** + * Excludes the feature methods declared in the superclasses of the annotated spec, so only the + * features declared in the annotated spec (and its subclasses) run. Fixture methods and helpers are + * still inherited, which lets a spec reuse an existing suite's setup without re-running its + * features. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +@ExtensionAnnotation(ExcludeInheritedFeaturesExtension.class) +public @interface ExcludeInheritedFeatures {} diff --git a/utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeaturesExtension.java b/utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeaturesExtension.java new file mode 100644 index 00000000000..0c809809194 --- /dev/null +++ b/utils/test-utils/src/main/java/datadog/trace/test/util/ExcludeInheritedFeaturesExtension.java @@ -0,0 +1,21 @@ +package datadog.trace.test.util; + +import org.spockframework.runtime.extension.IAnnotationDrivenExtension; +import org.spockframework.runtime.model.FeatureInfo; +import org.spockframework.runtime.model.SpecInfo; + +/** Handles specs annotated with {@link ExcludeInheritedFeatures}. */ +public class ExcludeInheritedFeaturesExtension + implements IAnnotationDrivenExtension { + + @Override + public void visitSpecAnnotation(final ExcludeInheritedFeatures annotation, final SpecInfo spec) { + final SpecInfo superSpec = spec.getSuperSpec(); + if (superSpec == null) { + return; + } + for (final FeatureInfo feature : superSpec.getAllFeatures()) { + feature.setExcluded(true); + } + } +} From 836bdf6e8224787123eb20af8e9821adda2e746d Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 15:01:59 +0200 Subject: [PATCH 4/6] review: pre-PR checks - Add cleanup: blocks restoring original IG callbacks after reset()+registerCallback() in play-appsec-2.5/2.6 and vertx-web-3.4/4.0/5.0 AppSec tests, preventing state leak across tests in the same forked JVM - Fix FQN inline okhttp3.Response -> import + Response in Play AppSec tests - Add missing blank line between Spock blocks for consistency --- .../play25/server/PlayServerAppSecTest.groovy | 126 +++++++++++ .../play26/server/PlayServerAppSecTest.groovy | 206 +++++++++++++++++- .../VertxHttpServerAppSecForkedTest.groovy | 151 +++++++++++++ .../VertxHttpServerAppSecForkedTest.groovy | 141 ++++++++++++ .../VertxHttpServerAppSecForkedTest.groovy | 141 ++++++++++++ 5 files changed, 764 insertions(+), 1 deletion(-) create mode 100644 dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy index c2d38efe9c4..e996a1da9e6 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy @@ -1,26 +1,47 @@ package datadog.trace.instrumentation.play25.server +import datadog.appsec.api.blocking.BlockingContentType +import datadog.appsec.api.blocking.BlockingException import datadog.trace.agent.test.base.HttpServer +import datadog.trace.agent.test.base.HttpServerTest.RbaFlow +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.instrumentation.play25.PlayRoutersScala import datadog.trace.test.util.ExcludeInheritedFeatures import groovy.json.JsonOutput import groovy.transform.CompileStatic import okhttp3.MediaType +import okhttp3.MultipartBody import okhttp3.RequestBody +import okhttp3.Response import spock.lang.Shared import java.util.concurrent.ExecutorService import java.util.concurrent.Executors +import java.util.function.BiFunction import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.api.gateway.Events.EVENTS +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.get import static org.junit.jupiter.api.Assumptions.assumeTrue @ExcludeInheritedFeatures class PlayServerAppSecTest extends PlayServerTest { + /** + * Message of the BlockingException thrown by the advice expected to block BODY_JSON: Java routers + * call Results.status(int, JsonNode) -> StatusHeader.sendJson(JsonNode, String). + */ + String expectedBlockingAdviceMessage() { + 'Blocked request (for StatusHeader/sendJson)' + } + /** * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), * reaching StatusHeaderSendJsonAdvice (Java routers) or ResultsStatusApplyAdvice (Scala routers). + * The controller span error pins which advice threw, so the block cannot come from another path. */ def 'test blocking on json response body'() { setup: @@ -47,6 +68,105 @@ class PlayServerAppSecTest extends PlayServerTest { rootSpan != null rootSpan.tags['http.status_code'] == 413 rootSpan.tags['appsec.blocked'] == 'true' + def controllerSpan = TEST_WRITER.get(0).find { + it.operationName.toString() == 'controller' + } + controllerSpan != null + controllerSpan.error + controllerSpan.tags['error.type'] == BlockingException.name + controllerSpan.tags['error.message'] == expectedBlockingAdviceMessage() + } + + /** + * Uploads a real multipart file part (no form fields, so the multipartFormData requestBodyProcessed + * branch is skipped) and blocks from the filenames or the files content callback. The BlockingException + * message pins which BodyParserHelpers branch committed the block. + */ + def 'test blocking on multipart file upload #variant'() { + setup: + installBlockingFileCallbacks() + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', filename, RequestBody.create(MediaType.parse('application/octet-stream'), content)) + .build() + def request = request(BODY_MULTIPART, 'POST', body).build() + + when: + def response = client.newCall(request).execute() + + then: + assertBlockedByBodyParser(response, expectedMessage) + + cleanup: + restoreFileCallbacks() + + where: + variant | filename | content | expectedMessage + 'filenames' | BLOCKED_FILENAME | 'file content' | 'Blocked request (multipart file upload)' + 'files content' | 'test.bin' | BLOCKED_CONTENT | 'Blocked request (multipart file upload content)' + } + + boolean assertBlockedByBodyParser(Response response, String expectedMessage) { + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + assert response.code() == 413 + assert response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + assert spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 413 && it.tags['appsec.blocked'] == 'true' + } != null + assert spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == expectedMessage + } != null + true + } + + static final String BLOCKED_FILENAME = 'block-me.php' + static final String BLOCKED_CONTENT = 'block-me-content' + + /** + * The shared HttpServerTest filenames/files content callbacks never block, and registering a + * second callback for the same event throws, so this spec's (per-spec) gateway slots are cleared + * and replaced with callbacks that block only for the BLOCKED_* markers. + */ + static void installBlockingFileCallbacks() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), blockingIf { List names -> names.contains(BLOCKED_FILENAME) }) + ss.reset(EVENTS.requestFilesContent()) + ss.registerCallback(EVENTS.requestFilesContent(), blockingIf { List contents -> contents.contains(BLOCKED_CONTENT) }) + } + + /** Restores the shared HttpServerTest filenames/files content callbacks replaced by installBlockingFileCallbacks(). */ + void restoreFileCallbacks() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext rqCtxt, List filenames -> + rqCtxt.traceSegment.setTagTop('request.body.filenames', filenames as String) + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilenames = filenames + context.uploadedFilenamesCallCount++ + rqCtxt.traceSegment.setTagTop('_dd.appsec.filenames.cb.calls', context.uploadedFilenamesCallCount) + Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + ss.reset(EVENTS.requestFilesContent()) + ss.registerCallback(EVENTS.requestFilesContent(), ({ RequestContext rqCtxt, List contents -> + rqCtxt.traceSegment.setTagTop('request.body.files_content', contents as String) + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilesContent = contents + Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } + + private static BiFunction, Flow> blockingIf(Closure shouldBlock) { + ({ RequestContext ctx, List values -> + shouldBlock(values) + ? new RbaFlow(new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction, Flow>) } } @@ -64,4 +184,10 @@ class PlayScalaAsyncServerAppSecTest extends PlayServerAppSecTest { executor = Executors.newCachedThreadPool() new PlayHttpServer(PlayRoutersScala.async(executor).asJava()) } + + /** Scala routers call Results.Ok(JsValue) -> Results$Status.apply(JsValue, Writeable). */ + @Override + String expectedBlockingAdviceMessage() { + 'Blocked request (for Results$Status/apply)' + } } diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy index 94f20208a6e..14b30303dae 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy @@ -1,18 +1,54 @@ package datadog.trace.instrumentation.play26.server +import datadog.appsec.api.blocking.BlockingContentType +import datadog.appsec.api.blocking.BlockingException +import datadog.trace.agent.test.base.HttpServer +import datadog.trace.agent.test.base.HttpServerTest.RbaFlow +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.test.util.ExcludeInheritedFeatures import groovy.json.JsonOutput +import groovy.transform.CompileStatic import okhttp3.MediaType +import okhttp3.MultipartBody import okhttp3.RequestBody +import okhttp3.Response +import play.BuiltInComponents +import play.api.http.Writeable$ +import play.api.libs.json.JsValue +import play.api.mvc.Results$ +import play.mvc.Http +import play.mvc.Result +import play.routing.Router +import play.routing.RoutingDsl + +import java.util.function.BiFunction +import java.util.function.Supplier import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_XML +import static datadog.trace.agent.test.base.HttpServerTest.controller +import static datadog.trace.api.gateway.Events.EVENTS +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.get +import static org.junit.jupiter.api.Assumptions.assumeTrue @ExcludeInheritedFeatures class PlayServerAppSecTest extends PlayServerTest { + /** + * Message of the BlockingException thrown by the advice expected to block BODY_JSON: Java routers + * call Results.status(int, JsonNode) -> StatusHeader.sendJson(JsonNode, String). + */ + String expectedBlockingAdviceMessage() { + 'Blocked request (for StatusHeader/sendJson)' + } + /** * Blocks from the JSON response body callback only ('body' is ignored by responseHeaderDone), - * reaching StatusHeaderSendJsonAdvice. + * reaching StatusHeaderSendJsonAdvice (Java routers) or ResultsStatusApplyAdvice (Scala Results). + * The controller span error pins which advice threw, so the block cannot come from another path. */ def 'test blocking on json response body'() { setup: @@ -38,5 +74,173 @@ class PlayServerAppSecTest extends PlayServerTest { rootSpan != null rootSpan.tags['http.status_code'] == 413 rootSpan.tags['appsec.blocked'] == 'true' + def controllerSpan = TEST_WRITER.get(0).find { + it.operationName.toString() == 'controller' + } + controllerSpan != null + controllerSpan.error + controllerSpan.tags['error.type'] == BlockingException.name + controllerSpan.tags['error.message'] == expectedBlockingAdviceMessage() + } + + /** Whether the server routes BODY_MULTIPART and BODY_XML (the shared Java routers do). */ + boolean testRequestBodyParserBlocking() { + true + } + + /** + * Uploads a real multipart file part (no form fields, so the multipartFormData requestBodyProcessed + * branch is skipped) and blocks from the filenames or the files content callback. The BlockingException + * message pins which BodyParserHelpers branch committed the block. + */ + def 'test blocking on multipart file upload #variant'() { + setup: + assumeTrue(testRequestBodyParserBlocking()) + installBlockingFileCallbacks() + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', filename, RequestBody.create(MediaType.parse('application/octet-stream'), content)) + .build() + def request = request(BODY_MULTIPART, 'POST', body).build() + + when: + def response = client.newCall(request).execute() + + then: + assertBlockedByBodyParser(response, expectedMessage) + + cleanup: + restoreFileCallbacks() + + where: + variant | filename | content | expectedMessage + 'filenames' | BLOCKED_FILENAME | 'file content' | 'Blocked request (multipart file upload)' + 'files content' | 'test.bin' | BLOCKED_CONTENT | 'Blocked request (multipart file upload content)' + } + + /** + * Blocks an XML body from requestBodyProcessed via the shared body-converted trigger. The body is + * parsed by the Scala tolerantXml parser wrapped by PlayBodyParsersInstrumentation.XmlAdvice, so the + * block comes from BodyParserHelpers.handleXml (not TolerantXmlInstrumentation's handleXmlDocument). + */ + def 'test blocking on xml request body'() { + setup: + assumeTrue(testRequestBodyParserBlocking()) + def request = request( + BODY_XML, 'POST', + RequestBody.create(MediaType.get('text/xml'), 'mytext')) + .header(IG_BODY_CONVERTED_HEADER, 'true') + .build() + + when: + def response = client.newCall(request).execute() + + then: + assertBlockedByBodyParser(response, 'Blocked request (for xml)') + } + + boolean assertBlockedByBodyParser(Response response, String expectedMessage) { + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + assert response.code() == 413 + assert response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + assert spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 413 && it.tags['appsec.blocked'] == 'true' + } != null + assert spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == expectedMessage + } != null + true + } + + static final String BLOCKED_FILENAME = 'block-me.php' + static final String BLOCKED_CONTENT = 'block-me-content' + + /** + * The shared HttpServerTest filenames/files content callbacks never block, and registering a + * second callback for the same event throws, so this spec's (per-spec) gateway slots are cleared + * and replaced with callbacks that block only for the BLOCKED_* markers. + */ + static void installBlockingFileCallbacks() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), blockingIf { List names -> names.contains(BLOCKED_FILENAME) }) + ss.reset(EVENTS.requestFilesContent()) + ss.registerCallback(EVENTS.requestFilesContent(), blockingIf { List contents -> contents.contains(BLOCKED_CONTENT) }) + } + + /** Restores the shared HttpServerTest filenames/files content callbacks replaced by installBlockingFileCallbacks(). */ + void restoreFileCallbacks() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext rqCtxt, List filenames -> + rqCtxt.traceSegment.setTagTop('request.body.filenames', filenames as String) + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilenames = filenames + context.uploadedFilenamesCallCount++ + rqCtxt.traceSegment.setTagTop('_dd.appsec.filenames.cb.calls', context.uploadedFilenamesCallCount) + Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + ss.reset(EVENTS.requestFilesContent()) + ss.registerCallback(EVENTS.requestFilesContent(), ({ RequestContext rqCtxt, List contents -> + rqCtxt.traceSegment.setTagTop('request.body.files_content', contents as String) + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilesContent = contents + Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } + + private static BiFunction, Flow> blockingIf(Closure shouldBlock) { + ({ RequestContext ctx, List values -> + shouldBlock(values) + ? new RbaFlow(new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction, Flow>) + } +} + +/** + * Serves BODY_JSON only through the Scala API (Results$Status.apply(JsValue, Writeable)), the sole + * real-server path to the 2.6 ResultsStatusApplyAdvice: the shared routers only use Java Results. + */ +class PlayScalaResultsServerAppSecTest extends PlayServerAppSecTest { + + @Override + HttpServer server() { + new PlayHttpServer(PlayScalaResultsServerAppSecTest.&scalaResultsRouter) + } + + @Override + String expectedBlockingAdviceMessage() { + 'Blocked request (for Results$Status/apply)' + } + + @Override + boolean testRequestBodyParserBlocking() { + false + } + + static Router scalaResultsRouter(BuiltInComponents components) { + RoutingDsl.fromComponents(components) + .POST(BODY_JSON.path).routeTo({ + -> + controller(BODY_JSON) { + JsValue json = Http.Context.current()._requestHeader().body.asJson().get() + scalaJsonResult(json) + } + } as Supplier) + .build() + } + + /** Statically compiled: a dynamic call site here hits a LinkageError on Results$Status. */ + @CompileStatic + private static Result scalaJsonResult(JsValue json) { + Results$.MODULE$.Status(BODY_JSON.status) + .apply(json, Writeable$.MODULE$.writeableOf_JsValue()) + .asJava() } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy new file mode 100644 index 00000000000..de1716c870e --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -0,0 +1,151 @@ +package server + +import datadog.appsec.api.blocking.BlockingContentType +import datadog.appsec.api.blocking.BlockingException +import datadog.trace.agent.test.base.HttpServerTest.RbaFlow +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.test.util.ExcludeInheritedFeatures +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody + +import java.util.function.BiFunction + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SESSION_ID +import static datadog.trace.api.gateway.Events.EVENTS +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.get + +@ExcludeInheritedFeatures +class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { + + /** + * SessionHandler creates a session and calls RoutingContextImpl.setSession(), reaching + * RoutingContextSessionAdvice, whose requestSession callback blocks only for BLOCK_SESSION_MARKER. + */ + def 'test blocking on session'() { + setup: + installBlockingSessionCallback() + def request = request(SESSION_ID, 'GET', null) + .header(IG_TEST_HEADER, BLOCK_SESSION_MARKER) + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 413 && it.tags['appsec.blocked'] == 'true' + } != null + spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == 'Blocked request (for session)' + } != null + + cleanup: + restoreSessionCallback() + } + + static final String BLOCK_SESSION_MARKER = 'block-session' + + /** + * The shared HttpServerTest requestSession callback never blocks, and registering a second + * callback for the same event throws, so this spec's (per-spec) gateway slot is cleared and + * replaced with a callback that blocks only when the IG test header carries BLOCK_SESSION_MARKER. + */ + static void installBlockingSessionCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestSession()) + ss.registerCallback(EVENTS.requestSession(), ({ RequestContext ctx, String sessionId -> + def context = ctx.getData(RequestContextSlot.APPSEC) + sessionId != null && context.matchingHeaderValue == BLOCK_SESSION_MARKER + ? new RbaFlow(new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction>)) + } + + /** Restores the shared HttpServerTest requestSession callback replaced by installBlockingSessionCallback(). */ + void restoreSessionCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestSession()) + ss.registerCallback(EVENTS.requestSession(), ({ RequestContext rqCtxt, String sessionId -> + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + if (context != null && sessionId != null) { + context.extraSpanName = 'appsec-span' + context.tags.put(IG_SESSION_ID_TAG, sessionId) + } + Flow.ResultFlow.empty() + } as BiFunction>)) + } + + /** + * Uploads a file part to BODY_MULTIPART, whose handler calls ctx.fileUploads(), reaching + * RoutingContextFilenamesAdvice, whose requestFilesFilenames callback blocks only for BLOCKED_FILENAME. + * The BlockingException message pins the filenames branch of FileUploadHelper.commitBlockingResponse. + */ + def 'test blocking on multipart file upload filename'() { + setup: + installBlockingFilenamesCallback() + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', BLOCKED_FILENAME, RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')) + .build() + def request = request(BODY_MULTIPART, 'POST', body).build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 403 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 403 && it.tags['appsec.blocked'] == 'true' + } != null + spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == 'Blocked request (multipart file upload)' + } != null + + cleanup: + restoreFilenamesCallback() + } + + static final String BLOCKED_FILENAME = 'block-me.php' + + /** + * The shared HttpServerTest requestFilesFilenames callback never blocks, and registering a second + * callback for the same event throws, so this spec's (per-spec) gateway slot is cleared and + * replaced with a callback that blocks only when BLOCKED_FILENAME is uploaded. + */ + static void installBlockingFilenamesCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext ctx, List filenames -> + filenames.contains(BLOCKED_FILENAME) + ? new RbaFlow(new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } + + /** Restores the shared HttpServerTest requestFilesFilenames callback replaced by installBlockingFilenamesCallback(). */ + void restoreFilenamesCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext rqCtxt, List filenames -> + rqCtxt.traceSegment.setTagTop('request.body.filenames', filenames as String) + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilenames = filenames + context.uploadedFilenamesCallCount++ + rqCtxt.traceSegment.setTagTop('_dd.appsec.filenames.cb.calls', context.uploadedFilenamesCallCount) + Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy index f3f1b878592..24c8e25c051 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -1,11 +1,24 @@ package server +import datadog.appsec.api.blocking.BlockingContentType +import datadog.appsec.api.blocking.BlockingException import datadog.trace.agent.test.base.HttpServer +import datadog.trace.agent.test.base.HttpServerTest.RbaFlow +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.test.util.ExcludeInheritedFeatures import okhttp3.MediaType +import okhttp3.MultipartBody import okhttp3.RequestBody +import java.util.function.BiFunction + import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SESSION_ID +import static datadog.trace.api.gateway.Events.EVENTS +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.get @ExcludeInheritedFeatures class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { @@ -36,6 +49,134 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { rootSpan.tags['http.status_code'] == 413 rootSpan.tags['appsec.blocked'] == 'true' } + + /** + * SessionHandler creates a session and calls RoutingContextImpl.setSession(), reaching + * RoutingContextSessionAdvice, whose requestSession callback blocks only for BLOCK_SESSION_MARKER. + */ + def 'test blocking on session'() { + setup: + installBlockingSessionCallback() + def request = request(SESSION_ID, 'GET', null) + .header(IG_TEST_HEADER, BLOCK_SESSION_MARKER) + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 413 && it.tags['appsec.blocked'] == 'true' + } != null + spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == 'Blocked request (for session)' + } != null + + cleanup: + restoreSessionCallback() + } + + static final String BLOCK_SESSION_MARKER = 'block-session' + + /** + * The shared HttpServerTest requestSession callback never blocks, and registering a second + * callback for the same event throws, so this spec's (per-spec) gateway slot is cleared and + * replaced with a callback that blocks only when the IG test header carries BLOCK_SESSION_MARKER. + */ + static void installBlockingSessionCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestSession()) + ss.registerCallback(EVENTS.requestSession(), ({ RequestContext ctx, String sessionId -> + def context = ctx.getData(RequestContextSlot.APPSEC) + sessionId != null && context.matchingHeaderValue == BLOCK_SESSION_MARKER + ? new RbaFlow(new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction>)) + } + + /** Restores the shared HttpServerTest requestSession callback replaced by installBlockingSessionCallback(). */ + void restoreSessionCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestSession()) + ss.registerCallback(EVENTS.requestSession(), ({ RequestContext rqCtxt, String sessionId -> + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + if (context != null && sessionId != null) { + context.extraSpanName = 'appsec-span' + context.tags.put(IG_SESSION_ID_TAG, sessionId) + } + Flow.ResultFlow.empty() + } as BiFunction>)) + } + + /** + * Uploads a file part to BODY_MULTIPART, whose handler calls ctx.fileUploads(), reaching + * RoutingContextFilenamesAdvice, whose requestFilesFilenames callback blocks only for BLOCKED_FILENAME. + * The BlockingException message pins the filenames branch of FileUploadHelper.commitBlockingResponse. + */ + def 'test blocking on multipart file upload filename'() { + setup: + installBlockingFilenamesCallback() + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', BLOCKED_FILENAME, RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')) + .build() + def request = request(BODY_MULTIPART, 'POST', body).build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 403 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 403 && it.tags['appsec.blocked'] == 'true' + } != null + spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == 'Blocked request (multipart file upload)' + } != null + + cleanup: + restoreFilenamesCallback() + } + + static final String BLOCKED_FILENAME = 'block-me.php' + + /** + * The shared HttpServerTest requestFilesFilenames callback never blocks, and registering a second + * callback for the same event throws, so this spec's (per-spec) gateway slot is cleared and + * replaced with a callback that blocks only when BLOCKED_FILENAME is uploaded. + */ + static void installBlockingFilenamesCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext ctx, List filenames -> + filenames.contains(BLOCKED_FILENAME) + ? new RbaFlow(new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } + + /** Restores the shared HttpServerTest requestFilesFilenames callback replaced by installBlockingFilenamesCallback(). */ + void restoreFilenamesCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext rqCtxt, List filenames -> + rqCtxt.traceSegment.setTagTop('request.body.filenames', filenames as String) + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilenames = filenames + context.uploadedFilenamesCallCount++ + rqCtxt.traceSegment.setTagTop('_dd.appsec.filenames.cb.calls', context.uploadedFilenamesCallCount) + Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } } class VertxHttpServerWorkerAppSecForkedTest extends VertxHttpServerAppSecForkedTest { diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy index f3f1b878592..24c8e25c051 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -1,11 +1,24 @@ package server +import datadog.appsec.api.blocking.BlockingContentType +import datadog.appsec.api.blocking.BlockingException import datadog.trace.agent.test.base.HttpServer +import datadog.trace.agent.test.base.HttpServerTest.RbaFlow +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.test.util.ExcludeInheritedFeatures import okhttp3.MediaType +import okhttp3.MultipartBody import okhttp3.RequestBody +import java.util.function.BiFunction + import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SESSION_ID +import static datadog.trace.api.gateway.Events.EVENTS +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.get @ExcludeInheritedFeatures class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { @@ -36,6 +49,134 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { rootSpan.tags['http.status_code'] == 413 rootSpan.tags['appsec.blocked'] == 'true' } + + /** + * SessionHandler creates a session and calls RoutingContextImpl.setSession(), reaching + * RoutingContextSessionAdvice, whose requestSession callback blocks only for BLOCK_SESSION_MARKER. + */ + def 'test blocking on session'() { + setup: + installBlockingSessionCallback() + def request = request(SESSION_ID, 'GET', null) + .header(IG_TEST_HEADER, BLOCK_SESSION_MARKER) + .build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 413 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 413 && it.tags['appsec.blocked'] == 'true' + } != null + spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == 'Blocked request (for session)' + } != null + + cleanup: + restoreSessionCallback() + } + + static final String BLOCK_SESSION_MARKER = 'block-session' + + /** + * The shared HttpServerTest requestSession callback never blocks, and registering a second + * callback for the same event throws, so this spec's (per-spec) gateway slot is cleared and + * replaced with a callback that blocks only when the IG test header carries BLOCK_SESSION_MARKER. + */ + static void installBlockingSessionCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestSession()) + ss.registerCallback(EVENTS.requestSession(), ({ RequestContext ctx, String sessionId -> + def context = ctx.getData(RequestContextSlot.APPSEC) + sessionId != null && context.matchingHeaderValue == BLOCK_SESSION_MARKER + ? new RbaFlow(new Flow.Action.RequestBlockingAction(413, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction>)) + } + + /** Restores the shared HttpServerTest requestSession callback replaced by installBlockingSessionCallback(). */ + void restoreSessionCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestSession()) + ss.registerCallback(EVENTS.requestSession(), ({ RequestContext rqCtxt, String sessionId -> + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + if (context != null && sessionId != null) { + context.extraSpanName = 'appsec-span' + context.tags.put(IG_SESSION_ID_TAG, sessionId) + } + Flow.ResultFlow.empty() + } as BiFunction>)) + } + + /** + * Uploads a file part to BODY_MULTIPART, whose handler calls ctx.fileUploads(), reaching + * RoutingContextFilenamesAdvice, whose requestFilesFilenames callback blocks only for BLOCKED_FILENAME. + * The BlockingException message pins the filenames branch of FileUploadHelper.commitBlockingResponse. + */ + def 'test blocking on multipart file upload filename'() { + setup: + installBlockingFilenamesCallback() + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', BLOCKED_FILENAME, RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')) + .build() + def request = request(BODY_MULTIPART, 'POST', body).build() + + when: + def response = client.newCall(request).execute() + + then: + response.code() == 403 + response.body().charStream().text.contains('"title":"You\'ve been blocked"') + TEST_WRITER.waitForTraces(1) + + def spans = TEST_WRITER.flatten() + spans.find { + it.parentId == 0 && it.tags['http.status_code'] == 403 && it.tags['appsec.blocked'] == 'true' + } != null + spans.find { + it.error && it.tags['error.type'] == BlockingException.name && it.tags['error.message'] == 'Blocked request (multipart file upload)' + } != null + + cleanup: + restoreFilenamesCallback() + } + + static final String BLOCKED_FILENAME = 'block-me.php' + + /** + * The shared HttpServerTest requestFilesFilenames callback never blocks, and registering a second + * callback for the same event throws, so this spec's (per-spec) gateway slot is cleared and + * replaced with a callback that blocks only when BLOCKED_FILENAME is uploaded. + */ + static void installBlockingFilenamesCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext ctx, List filenames -> + filenames.contains(BLOCKED_FILENAME) + ? new RbaFlow(new Flow.Action.RequestBlockingAction(403, BlockingContentType.JSON)) + : Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } + + /** Restores the shared HttpServerTest requestFilesFilenames callback replaced by installBlockingFilenamesCallback(). */ + void restoreFilenamesCallback() { + def ss = get().getSubscriptionService(RequestContextSlot.APPSEC) + ss.reset(EVENTS.requestFilesFilenames()) + ss.registerCallback(EVENTS.requestFilesFilenames(), ({ RequestContext rqCtxt, List filenames -> + rqCtxt.traceSegment.setTagTop('request.body.filenames', filenames as String) + def context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilenames = filenames + context.uploadedFilenamesCallCount++ + rqCtxt.traceSegment.setTagTop('_dd.appsec.filenames.cb.calls', context.uploadedFilenamesCallCount) + Flow.ResultFlow.empty() + } as BiFunction, Flow>)) + } } class VertxHttpServerWorkerAppSecForkedTest extends VertxHttpServerAppSecForkedTest { From 68fbefa12ab3945d0c66c566fb5330d6e6a816ca Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 16:37:39 +0200 Subject: [PATCH 5/6] review: fix NPE in Play AppSec multipart test (data-driven where: block) - Rename local `request` variable to `req` in play-appsec-2.5 and play-appsec-2.6 PlayServerAppSecTest's multipart file upload test, which shadowed the inherited `request()` method once Spock's `where:` block hoisted the local variable declaration. --- .../instrumentation/play25/server/PlayServerAppSecTest.groovy | 4 ++-- .../instrumentation/play26/server/PlayServerAppSecTest.groovy | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy index e996a1da9e6..9fd29bbaa8e 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.5/src/test/groovy/datadog/trace/instrumentation/play25/server/PlayServerAppSecTest.groovy @@ -89,10 +89,10 @@ class PlayServerAppSecTest extends PlayServerTest { .setType(MultipartBody.FORM) .addFormDataPart('file', filename, RequestBody.create(MediaType.parse('application/octet-stream'), content)) .build() - def request = request(BODY_MULTIPART, 'POST', body).build() + def req = request(BODY_MULTIPART, 'POST', body).build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: assertBlockedByBodyParser(response, expectedMessage) diff --git a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy index 14b30303dae..3dfec7124cb 100644 --- a/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy +++ b/dd-java-agent/instrumentation/play/play-appsec-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerAppSecTest.groovy @@ -101,10 +101,10 @@ class PlayServerAppSecTest extends PlayServerTest { .setType(MultipartBody.FORM) .addFormDataPart('file', filename, RequestBody.create(MediaType.parse('application/octet-stream'), content)) .build() - def request = request(BODY_MULTIPART, 'POST', body).build() + def req = request(BODY_MULTIPART, 'POST', body).build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: assertBlockedByBodyParser(response, expectedMessage) From a4b510447e96d14cc0667f646103350bf29efd78 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 19:26:59 +0200 Subject: [PATCH 6/6] review: fix NPE in Vertx AppSec blocking tests with cleanup: block Local variable `request` shadowed the inherited request() method, causing Groovy to desugar the call to request.call(args) on the null local var. Unlike the earlier play-appsec fix, the trigger here is a cleanup: block (not a where: block): sibling tests without cleanup: using the same def request = request(...) pattern were unaffected. Renamed to req in vertx-web-3.4/4.0/5.0. --- .../groovy/server/VertxHttpServerAppSecForkedTest.groovy | 8 ++++---- .../groovy/server/VertxHttpServerAppSecForkedTest.groovy | 8 ++++---- .../groovy/server/VertxHttpServerAppSecForkedTest.groovy | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy index de1716c870e..5d5910cd838 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -28,12 +28,12 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { def 'test blocking on session'() { setup: installBlockingSessionCallback() - def request = request(SESSION_ID, 'GET', null) + def req = request(SESSION_ID, 'GET', null) .header(IG_TEST_HEADER, BLOCK_SESSION_MARKER) .build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: response.code() == 413 @@ -96,10 +96,10 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { .setType(MultipartBody.FORM) .addFormDataPart('file', BLOCKED_FILENAME, RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')) .build() - def request = request(BODY_MULTIPART, 'POST', body).build() + def req = request(BODY_MULTIPART, 'POST', body).build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: response.code() == 403 diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy index 24c8e25c051..b0a1c7d93f3 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -57,12 +57,12 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { def 'test blocking on session'() { setup: installBlockingSessionCallback() - def request = request(SESSION_ID, 'GET', null) + def req = request(SESSION_ID, 'GET', null) .header(IG_TEST_HEADER, BLOCK_SESSION_MARKER) .build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: response.code() == 413 @@ -125,10 +125,10 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { .setType(MultipartBody.FORM) .addFormDataPart('file', BLOCKED_FILENAME, RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')) .build() - def request = request(BODY_MULTIPART, 'POST', body).build() + def req = request(BODY_MULTIPART, 'POST', body).build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: response.code() == 403 diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy index 24c8e25c051..b0a1c7d93f3 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerAppSecForkedTest.groovy @@ -57,12 +57,12 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { def 'test blocking on session'() { setup: installBlockingSessionCallback() - def request = request(SESSION_ID, 'GET', null) + def req = request(SESSION_ID, 'GET', null) .header(IG_TEST_HEADER, BLOCK_SESSION_MARKER) .build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: response.code() == 413 @@ -125,10 +125,10 @@ class VertxHttpServerAppSecForkedTest extends VertxHttpServerForkedTest { .setType(MultipartBody.FORM) .addFormDataPart('file', BLOCKED_FILENAME, RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')) .build() - def request = request(BODY_MULTIPART, 'POST', body).build() + def req = request(BODY_MULTIPART, 'POST', body).build() when: - def response = client.newCall(request).execute() + def response = client.newCall(req).execute() then: response.code() == 403