From 2c99b3ee3914d23f64e0f4de9cd8f0f7636f05b0 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 27 Aug 2026 13:47:34 +0200 Subject: [PATCH 1/3] Add block-outcome telemetry for Netty blocking enforcement failures Report failed AppSec block-response commits to WafMetricCollector so appsec.waf.requests carries an accurate block_failure tag, scoped to Netty as the first framework. AppSecContext gains a module-boundary-safe reportBlockFailure() that AppSecRequestContext delegates to the existing setWafRequestBlockFailure() field. --- .../appsec/gateway/AppSecRequestContext.java | 5 + .../ddwaf/WAFModuleSpecification.groovy | 89 +++++++++ .../netty41/NettyMultipartHelper.java | 16 +- .../server/MaybeBlockResponseHandler.java | 1 + .../NettyMultipartHelperBlockFailureTest.java | 169 ++++++++++++++++++ .../trace/api/appsec/AppSecContext.java | 3 + 6 files changed, 279 insertions(+), 4 deletions(-) create mode 100644 dd-java-agent/instrumentation/netty/netty-4.1/src/test/java/datadog/trace/instrumentation/netty41/NettyMultipartHelperBlockFailureTest.java diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java index 3a3a65002fe..b43249a1988 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java @@ -281,6 +281,11 @@ public boolean isWafRequestBlockFailure() { return wafRequestBlockFailure; } + @Override + public void reportBlockFailure() { + setWafRequestBlockFailure(); + } + public void setWafRateLimited() { this.wafRateLimited = true; } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy index 1c3bb0b32ff..17ff4987959 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy @@ -1654,6 +1654,95 @@ class WAFModuleSpecification extends DDSpecification { 0 * _ } + @Unroll + void 'raspRuleMatch reports blocked=#expectedBlocked after the action processing loop (#userAgent)'() { + setup: + // Two RASP-matching rules on the same address: one blocks, the other only asks for a stack + // trace. The `blocked` flag reported to telemetry must be resolved AFTER the action processing + // loop has run, since that loop is what turns a match into an actual block (or not). + def rulesConfig = [ + version : '2.1', + metadata: [rules_version: '1.2.7'], + rules : [ + [ + id : 'rasp-blocking-rule', + name : 'RASP blocking rule', + tags : [ + type : 'sql_injection', + category: 'exploit_attempt' + ], + conditions: [ + [ + parameters: [ + inputs: [ + [ + address : 'server.request.headers.no_cookies', + key_path: ['user-agent'] + ] + ], + regex : '^RaspBlocking' + ], + operator : 'match_regex' + ] + ], + on_match : ['block'] + ], + [ + id : 'rasp-stack-only-rule', + name : 'RASP stack-generation-only rule', + tags : [ + type : 'sql_injection', + category: 'exploit_attempt' + ], + conditions: [ + [ + parameters: [ + inputs: [ + [ + address : 'server.request.headers.no_cookies', + key_path: ['user-agent'] + ] + ], + regex : '^RaspStackOnly' + ], + operator : 'match_regex' + ] + ], + on_match : ['stack_trace'] + ] + ] + ] + def raspGwCtx = new GatewayContext(false, RuleType.SQL_INJECTION) + + when: + initialRuleAddWithMap(rulesConfig) + wafModule.applyConfig(reconf) + + then: + 1 * wafMetricCollector.wafInit(Waf.LIB_VERSION, _, true) + 1 * wafMetricCollector.wafUpdates(_, true) + 1 * reconf.reloadSubscriptions() + + when: + def flow = new ChangeableFlow() + def bundle = MapDataBundle.of(KnownAddresses.HEADERS_NO_COOKIES, + new CaseInsensitiveMap>(['user-agent': userAgent])) + dataListener.onDataAvailable(flow, ctx, bundle, raspGwCtx) + ctx.closeWafContext() + + then: + flow.blocking == expectedBlocked + 1 * ctx.setRaspMatched(true) + 1 * wafMetricCollector.raspRuleEval(RuleType.SQL_INJECTION) + 1 * wafMetricCollector.raspRuleMatch(RuleType.SQL_INJECTION, expectedBlocked) + 0 * wafMetricCollector.raspRuleMatch(RuleType.SQL_INJECTION, !expectedBlocked) + + where: + userAgent | expectedBlocked + 'RaspBlocking/v1' | true + 'RaspStackOnly/v1' | false + } + void 'test raspErrorCode metric is increased when waf call throws #wafErrorCode '() { setup: ChangeableFlow flow = Mock() diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyMultipartHelper.java b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyMultipartHelper.java index 95a0c6292e7..f09df5944c5 100644 --- a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyMultipartHelper.java +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyMultipartHelper.java @@ -2,9 +2,11 @@ import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.Config; +import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.api.http.MultipartContentDecoder; import io.netty.buffer.ByteBuf; import io.netty.handler.codec.http.multipart.Attribute; @@ -86,9 +88,10 @@ public static String readContent(FileUpload fileUpload) { } /** - * Checks if the flow action is a blocking action and, if so, commits the blocking response. - * Returns a {@link BlockingException} to be re-thrown by the advice, or {@code null} if no - * blocking action was taken. + * Checks if the flow action is a blocking action and, if so, commits the blocking response. If + * the commit fails, reports the failure to {@link AppSecContext#reportBlockFailure()}. Returns a + * {@link BlockingException} to be re-thrown by the advice, or {@code null} if no blocking action + * was taken. */ public static BlockingException tryBlock(RequestContext ctx, Flow flow, String message) { Flow.Action action = flow.getAction(); @@ -96,7 +99,12 @@ public static BlockingException tryBlock(RequestContext ctx, Flow flow, St Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; BlockResponseFunction brf = ctx.getBlockResponseFunction(); if (brf != null) { - brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + if (!brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba)) { + Object rawAppSecCtx = ctx.getData(RequestContextSlot.APPSEC); + if (rawAppSecCtx instanceof AppSecContext) { + ((AppSecContext) rawAppSecCtx).reportBlockFailure(); + } + } return new BlockingException(message); } } diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java index 11782d59e02..82bede8dcd1 100644 --- a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java @@ -125,6 +125,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) thr .addListener( fut -> { if (!fut.isSuccess()) { + // known gap: this failure is not reported to AppSec block-failure telemetry log.warn("Write of blocking response failed", fut.cause()); } else { log.debug("Write of blocking response succeeded"); diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/test/java/datadog/trace/instrumentation/netty41/NettyMultipartHelperBlockFailureTest.java b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/java/datadog/trace/instrumentation/netty41/NettyMultipartHelperBlockFailureTest.java new file mode 100644 index 00000000000..c0c9dbae5f2 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/java/datadog/trace/instrumentation/netty41/NettyMultipartHelperBlockFailureTest.java @@ -0,0 +1,169 @@ +package datadog.trace.instrumentation.netty41; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.appsec.AppSecContext; +import datadog.trace.api.gateway.BlockResponseFunction; +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.ClientIpAddressData; +import java.util.Map; +import java.util.function.Function; +import org.junit.jupiter.api.Test; + +/** + * Covers the {@code tryBlock() -> AppSecContext.reportBlockFailure()} path. Hand-written test + * doubles are used because Mockito is only on this module's test runtime classpath, not its test + * compile classpath. + */ +class NettyMultipartHelperBlockFailureTest { + + private static final Flow.Action.RequestBlockingAction RBA = + new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO); + + @Test + void reportsBlockFailureWhenBlockingResponseCannotBeCommitted() { + CountingAppSecContext appSecCtx = new CountingAppSecContext(); + TestRequestContext ctx = + new TestRequestContext(new TestBlockResponseFunction(false), appSecCtx); + + BlockingException exception = NettyMultipartHelper.tryBlock(ctx, blockingFlow(), "blocked!"); + + assertNotNull(exception); + assertEquals("blocked!", exception.getMessage()); + assertEquals(1, appSecCtx.blockFailures); + assertSame(RBA, ctx.brf.lastAction); + assertSame(ctx.traceSegment, ctx.brf.lastSegment); + } + + @Test + void doesNotReportBlockFailureWhenBlockingResponseIsCommitted() { + CountingAppSecContext appSecCtx = new CountingAppSecContext(); + TestRequestContext ctx = new TestRequestContext(new TestBlockResponseFunction(true), appSecCtx); + + BlockingException exception = NettyMultipartHelper.tryBlock(ctx, blockingFlow(), "blocked!"); + + assertNotNull(exception); + assertEquals("blocked!", exception.getMessage()); + assertEquals(0, appSecCtx.blockFailures); + } + + @Test + void doesNotThrowWhenAppSecSlotDoesNotHoldAnAppSecContext() { + TestRequestContext nullSlot = + new TestRequestContext(new TestBlockResponseFunction(false), null); + assertNotNull(NettyMultipartHelper.tryBlock(nullSlot, blockingFlow(), "blocked!")); + + TestRequestContext foreignSlot = + new TestRequestContext(new TestBlockResponseFunction(false), "not an AppSecContext"); + assertNotNull(NettyMultipartHelper.tryBlock(foreignSlot, blockingFlow(), "blocked!")); + } + + private static Flow blockingFlow() { + return new Flow() { + @Override + public Action getAction() { + return RBA; + } + + @Override + public Void getResult() { + return null; + } + }; + } + + private static final class CountingAppSecContext implements AppSecContext { + private int blockFailures; + + @Override + public boolean isManuallyKept() { + return false; + } + + @Override + public void reportBlockFailure() { + blockFailures++; + } + } + + private static final class TestBlockResponseFunction implements BlockResponseFunction { + private final boolean committed; + private TraceSegment lastSegment; + private Flow.Action.RequestBlockingAction lastAction; + + private TestBlockResponseFunction(boolean committed) { + this.committed = committed; + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, Flow.Action.RequestBlockingAction rba) { + this.lastAction = rba; + return BlockResponseFunction.super.tryCommitBlockingResponse(segment, rba); + } + + @Override + public boolean tryCommitBlockingResponse( + TraceSegment segment, + int statusCode, + BlockingContentType templateType, + Map extraHeaders, + String securityResponseId) { + this.lastSegment = segment; + return committed; + } + } + + private static final class TestRequestContext implements RequestContext { + private final TestBlockResponseFunction brf; + private final Object appSecData; + private final TraceSegment traceSegment = TraceSegment.NoOp.INSTANCE; + + private TestRequestContext(TestBlockResponseFunction brf, Object appSecData) { + this.brf = brf; + this.appSecData = appSecData; + } + + @SuppressWarnings("unchecked") + @Override + public T getData(RequestContextSlot slot) { + return slot == RequestContextSlot.APPSEC ? (T) appSecData : null; + } + + @Override + public TraceSegment getTraceSegment() { + return traceSegment; + } + + @Override + public void setBlockResponseFunction(BlockResponseFunction blockResponseFunction) {} + + @Override + public BlockResponseFunction getBlockResponseFunction() { + return brf; + } + + @Override + public T getOrCreateMetaStructTop(String key, Function defaultValue) { + return null; + } + + @Override + public void setClientIpAddressData(ClientIpAddressData clientIpAddressData) {} + + @Override + public ClientIpAddressData getClientIpAddressData() { + return null; + } + + @Override + public void close() {} + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecContext.java b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecContext.java index 53cd0ab284a..8d45f83d413 100644 --- a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecContext.java +++ b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecContext.java @@ -3,4 +3,7 @@ /** Minimal view of the AppSec request context accessible across module boundaries. */ public interface AppSecContext { boolean isManuallyKept(); + + /** Reports that an attempted AppSec block could not be committed or enforced. */ + void reportBlockFailure(); } From cdd1140d3ddeead824fa8b0aa7e5a8c3d2a8c191 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 27 Aug 2026 14:29:50 +0200 Subject: [PATCH 2/3] Fix spurious block_failure on repeated Netty multipart block attempts NettyBlockResponseFunction.tryCommitBlockingResponse() now tracks whether a blocking response was already initiated for the current request. HttpPostRequestDecoderInstrumentation's advice can invoke tryBlock() multiple times across separate decoder invocations for the same request (e.g. one per multipart chunk); once an earlier call already committed the block successfully, ServerRequestContext.isPending() returns false for later calls, which previously caused a spurious block_failure to be reported even though the block had actually succeeded. --- .../server/NettyHttpServerDecorator.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java index 81d94995d05..defde3598e5 100644 --- a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java @@ -122,6 +122,7 @@ public static class NettyBlockResponseFunction implements BlockResponseFunction private final HttpVersion protocolVersion; private final String acceptHeader; private final ServerRequestContext serverContext; + private volatile boolean blockingResponseInitiated; public NettyBlockResponseFunction( ChannelPipeline pipeline, @@ -140,9 +141,22 @@ public boolean tryCommitBlockingResponse( BlockingContentType templateType, Map extraHeaders, String securityResponseId) { + // A single request can trigger multiple blocking evaluations (e.g. one per multipart + // chunk). Once a block has already been initiated, the response queue entry backing + // isPending() may have already been consumed by that earlier, successful commit — treat + // later calls as already handled rather than re-evaluating and reporting a spurious + // block_failure for a block that actually succeeded. + if (blockingResponseInitiated) { + return true; + } if (pipeline.channel().eventLoop().inEventLoop()) { - return commitBlockingResponse( - segment, statusCode, templateType, extraHeaders, securityResponseId); + boolean committed = + commitBlockingResponse( + segment, statusCode, templateType, extraHeaders, securityResponseId); + if (committed) { + blockingResponseInitiated = true; + } + return committed; } try { @@ -153,6 +167,7 @@ public boolean tryCommitBlockingResponse( () -> commitBlockingResponse( segment, statusCode, templateType, extraHeaders, securityResponseId)); + blockingResponseInitiated = true; return true; } catch (RuntimeException rte) { log.warn("Failed scheduling blocking handler", rte); From 4a2bd4db0eb2fa89fc488cd2fd7c98b32e44ccb7 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 27 Aug 2026 15:58:23 +0200 Subject: [PATCH 3/3] Fix async blocking flag set before commit confirmation NettyBlockResponseFunction.tryCommitBlockingResponse() marked blockingResponseInitiated = true immediately after scheduling the off-event-loop commitBlockingResponse() call, without waiting for it to actually run. If that async commit failed once executed, the failure was silently swallowed and every later tryBlock() call for the same request would short-circuit to true, hiding the failure from block_failure telemetry. The flag is now set only inside the scheduled task, after commitBlockingResponse() actually returns true, matching the same-thread branch's existing behavior. --- .../netty41/server/NettyHttpServerDecorator.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java index defde3598e5..ddd56814d0c 100644 --- a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java @@ -164,10 +164,12 @@ public boolean tryCommitBlockingResponse( .channel() .eventLoop() .execute( - () -> - commitBlockingResponse( - segment, statusCode, templateType, extraHeaders, securityResponseId)); - blockingResponseInitiated = true; + () -> { + if (commitBlockingResponse( + segment, statusCode, templateType, extraHeaders, securityResponseId)) { + blockingResponseInitiated = true; + } + }); return true; } catch (RuntimeException rte) { log.warn("Failed scheduling blocking handler", rte);