diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java index a59aafc2710..614aa40c0ff 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java @@ -288,6 +288,34 @@ void appSecCallbacksAreNotInvokedWhenAppSecIsDisabled() throws IOException { assertTraces(trace(span().type(DDSpanTypes.SERVERLESS).error(false))); } + @Test + void appSecIsSkippedAndReportedUnsupportedForNonHttpEvent() throws IOException { + String eventJson = "{\"Records\": [{\"eventSource\": \"aws:sqs\", \"body\": \"hello\"}]}"; + + ByteArrayInputStream input = + new ByteArrayInputStream(eventJson.getBytes(StandardCharsets.UTF_8)); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + new HandlerStreaming().handleRequest(input, output, newContext()); + + assertFalse(appSecStarted); + assertNull(capturedMethod); + assertNull(capturedPath); + assertTrue(capturedHeaders.isEmpty()); + assertNull(capturedBody); + assertFalse(appSecEnded); + assertNull(capturedResponseStatus); + // Tag matching is exhaustive, so this also asserts the span carries no http.* tag + assertTraces( + trace( + span() + .type(DDSpanTypes.SERVERLESS) + .error(false) + .tags( + defaultTags(), + tag("request_id", is(REQUEST_ID)), + tag("_dd.appsec.unsupported_event_type", is(1))))); + } + @Test void responseCallbacksAreInvokedForJsonEncodedResponse() throws IOException { String eventJson = @@ -382,7 +410,8 @@ void responseCallbacksSkipNonApiGatewayResponseForNonHttpEvent() throws IOExcept assertTrue(capturedResponseHeaders.isEmpty()); assertNull(capturedResponseBody); assertFalse(responseHeaderDoneCalled); - assertTrue(appSecEnded); + // AppSec skipped the invocation entirely, so there is no request context to end + assertFalse(appSecEnded); assertTraces(trace(span().type(DDSpanTypes.SERVERLESS).error(false))); } diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java index 84d25ccdf8e..fdc8fe67637 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java @@ -57,6 +57,9 @@ public class LambdaAppSecHandler { private static final Logger log = LoggerFactory.getLogger(LambdaAppSecHandler.class); private static final RatelimitedLogger rlLog = new RatelimitedLogger(log, 5, TimeUnit.MINUTES); + /** Marks an invocation AppSec did not process because the trigger is not HTTP. */ + private static final String UNSUPPORTED_EVENT_TYPE_METRIC = "_dd.appsec.unsupported_event_type"; + // Carries the detected trigger type from processRequestStart to processResponseData within the // same Lambda invocation. Cleared in processRequestEnd. private static final ThreadLocal CURRENT_TRIGGER_TYPE = new ThreadLocal<>(); @@ -68,7 +71,8 @@ public class LambdaAppSecHandler { * * @param event the Lambda event object * @return a {@link TagContext} carrying the AppSec request context and the HTTP tags, or null if - * AppSec is disabled, the event is not a parseable payload, or processing fails + * AppSec is disabled, the trigger is not HTTP, the event is not a parseable payload, or + * processing fails */ public static AgentSpanContext processRequestStart(Object event) { if (!ActiveSubsystems.APPSEC_ACTIVE) { @@ -91,6 +95,11 @@ public static AgentSpanContext processRequestStart(Object event) { return null; } CURRENT_TRIGGER_TYPE.set(eventData.triggerType); + if (!eventData.triggerType.isHttp()) { + log.debug("Trigger type {} is not HTTP, skipping AppSec processing", eventData.triggerType); + // unsupported event metric is added on request end since span doesn't exist yet + return null; + } // v2 payloads carry the request line verbatim; the others expose the path and a decoded // parameter map only, so the query string has to be rebuilt from them String fullPath = eventData.rawUri; @@ -100,7 +109,7 @@ public static AgentSpanContext processRequestStart(Object event) { LambdaURIDataAdapter uriAdapter = new LambdaURIDataAdapter(fullPath, eventData.headers, eventData.host); AgentSpanContext context = processAppSecRequestData(eventData, uriAdapter); - if (context instanceof TagContext && eventData.triggerType.isHttp()) { + if (context instanceof TagContext) { applyHttpTags((TagContext) context, eventData, uriAdapter); } return context; @@ -117,12 +126,20 @@ public static AgentSpanContext processRequestStart(Object event) { * @param span the current span */ public static void processRequestEnd(AgentSpan span) { + LambdaTriggerType triggerType = CURRENT_TRIGGER_TYPE.get(); CURRENT_TRIGGER_TYPE.remove(); if (!ActiveSubsystems.APPSEC_ACTIVE || span == null) { return; } + // A null trigger type means processRequestStart never ran, so the invocation was not analysed + // at all, which is not the same as an unsupported trigger. + if (triggerType != null && !triggerType.isHttp()) { + span.setMetric(UNSUPPORTED_EVENT_TYPE_METRIC, 1); + return; + } + RequestContext requestContext = span.getRequestContext(); if (requestContext != null) { AgentTracer.TracerAPI tracer = AgentTracer.get(); diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java index fdb03e6edbb..1848e78b620 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaEventParser.java @@ -94,8 +94,9 @@ static LambdaRequestData parseEvent(String json) { case ALB_MULTI_VALUE: return extractAlbData(event, triggerType); default: - log.debug("Unknown trigger type, attempting generic extraction"); - return extractGenericData(event); + // Unsupported trigger: AppSec skips the invocation entirely, so there is nothing to + // extract. The trigger type is carried by the caller, not by this result. + return LambdaRequestData.EMPTY; } } catch (Exception e) { log.debug("Failed to parse event data from JSON", e); @@ -444,69 +445,6 @@ private static LambdaRequestData extractAlbData( null); } - /** Generic data extraction for unknown trigger types (fallback) */ - private static LambdaRequestData extractGenericData(Map event) { - Map headers = extractHeadersWithCookies(event); - Map pathParameters = extractPathParameters(event.get("pathParameters")); - Map> queryParameters = - extractQueryParameters(event.get("queryStringParameters")); - Object body = extractBody(event); - - String method = null; - String path = null; - String sourceIp = null; - - // Try to extract from requestContext if available - Object requestContextObj = event.get("requestContext"); - if (requestContextObj instanceof Map) { - Map requestContext = (Map) requestContextObj; - - Object httpObj = requestContext.get("http"); - if (httpObj instanceof Map) { - Map http = (Map) httpObj; - method = (String) http.get("method"); - path = (String) http.get("path"); - sourceIp = (String) http.get("sourceIp"); - } else { - Object methodObj = requestContext.get("httpMethod"); - if (methodObj != null) { - method = String.valueOf(methodObj); - } - - Object identityObj = requestContext.get("identity"); - if (identityObj instanceof Map) { - Map identity = (Map) identityObj; - sourceIp = (String) identity.get("sourceIp"); - } - } - } - - // Try root level fields - if (method == null) { - Object methodObj = event.get("httpMethod"); - if (methodObj != null) { - method = String.valueOf(methodObj); - } - } - if (path == null) { - Object pathObj = event.get("path"); - if (pathObj != null) { - path = String.valueOf(pathObj); - } - } - - return new LambdaRequestData( - headers, - method, - path, - sourceIp, - null, - LambdaTriggerType.UNKNOWN, - pathParameters, - queryParameters, - body); - } - /** * Looks a header up in a map produced by {@link #extractHeaders}, whose keys are already * lowercased, so {@code lowerCaseName} must be lowercase for a match. @@ -790,8 +728,22 @@ enum LambdaTriggerType { LAMBDA_URL, // Lambda Function URL UNKNOWN; // Unknown or unsupported trigger + /** + * Whitelist rather than {@code != UNKNOWN} so a trigger type added later defaults to non-HTTP, + * and therefore to being skipped by AppSec, until it is deliberately listed here. + */ boolean isHttp() { - return this != UNKNOWN; + switch (this) { + case API_GATEWAY_V1_REST: + case API_GATEWAY_V2_HTTP: + case API_GATEWAY_V2_WEBSOCKET: + case ALB: + case ALB_MULTI_VALUE: + case LAMBDA_URL: + return true; + default: + return false; + } } } diff --git a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java index 431fcf833be..33e68f528e2 100644 --- a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java @@ -21,6 +21,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import datadog.trace.api.Config; @@ -138,6 +139,27 @@ void processRequestStartReturnsNullForMalformedJson() { assertNull(LambdaAppSecHandler.processRequestStart(event)); } + @Test + @SuppressWarnings("unchecked") + void processRequestStartSkipsAppSecForNonHttpTrigger() { + String sqsEvent = + "{\"Records\": [{\"eventSource\": \"aws:sqs\", \"body\": \"hello\"," + + " \"messageAttributes\": {}}]}"; + ByteArrayInputStream event = createInputStream(sqsEvent); + + Supplier> requestStartedCallback = mock(Supplier.class); + CallbackProvider mockCallbackProvider = mock(CallbackProvider.class); + when(mockCallbackProvider.getCallback(EVENTS.requestStarted())) + .thenReturn(requestStartedCallback); + AgentTracer.TracerAPI mockTracer = mock(AgentTracer.TracerAPI.class); + when(mockTracer.getCallbackProvider(RequestContextSlot.APPSEC)) + .thenReturn(mockCallbackProvider); + AgentTracer.forceRegister(mockTracer); + + assertNull(LambdaAppSecHandler.processRequestStart(event)); + verifyNoInteractions(requestStartedCallback); + } + @Test void streamCanBeReadMultipleTimesAfterProcessing() throws IOException { String jsonData = "{\"test\": \"data\", \"requestContext\": {\"httpMethod\": \"GET\"}}"; @@ -903,47 +925,11 @@ void handlesBodyWithSpecialCharacters() { } // ============================================================================ - // Generic Data Extraction Tests + // Partial Payload Extraction Tests // ============================================================================ @Test - void extractsDataFromUnknownTriggerTypeUsingGenericExtraction() { - String eventJson = - "{" - + "\"path\": \"/generic/path\"," - + "\"httpMethod\": \"PATCH\"," - + "\"headers\": {\"x-custom-header\": \"generic-value\"}," - + "\"unknownField\": \"should be ignored\"," - + "\"requestContext\": {\"identity\": {\"sourceIp\": \"203.0.113.1\"}}" - + "}"; - ByteArrayInputStream event = createInputStream(eventJson); - - String[] capturedMethod = {null}; - String[] capturedPath = {null}; - Map capturedHeaders = new HashMap<>(); - String[] capturedSourceIp = {null}; - - setupMockCallbacks( - new Callbacks() - .onMethodUri( - (method, uri) -> { - capturedMethod[0] = method; - capturedPath[0] = uri.path(); - }) - .onHeader(capturedHeaders::put) - .onSocketAddress((ip, port) -> capturedSourceIp[0] = ip)); - - AgentSpanContext result = LambdaAppSecHandler.processRequestStart(event); - - assertNotNull(result); - assertEquals("PATCH", capturedMethod[0]); - assertEquals("/generic/path", capturedPath[0]); - assertEquals("generic-value", capturedHeaders.get("x-custom-header")); - assertEquals("203.0.113.1", capturedSourceIp[0]); - } - - @Test - void extractsDataFromUnknownTriggerWithHttpInRequestContext() { + void extractsDataFromLambdaUrlWithHttpInRequestContext() { String eventJson = "{" + "\"requestContext\": {" @@ -974,7 +960,7 @@ void extractsDataFromUnknownTriggerWithHttpInRequestContext() { } @Test - void genericExtractionUsesHttpMethodFromRequestContext() { + void extractsHttpMethodFromRequestContextOfApiGatewayV1Payload() { String eventJson = "{\"path\": \"/ctx-method\", \"requestContext\": {\"httpMethod\": \"DELETE\"}}"; ByteArrayInputStream event = createInputStream(eventJson); @@ -1059,6 +1045,7 @@ void processRequestEndDoesNothingWhenAppSecIsDisabled() { @Test void processRequestEndDoesNothingWhenSpanHasNoRequestContext() { + LambdaAppSecHandler.setCurrentTriggerType(LambdaTriggerType.API_GATEWAY_V1_REST); AgentSpan span = mock(AgentSpan.class); when(span.getRequestContext()).thenReturn(null); LambdaAppSecHandler.processRequestEnd(span); @@ -1068,6 +1055,7 @@ void processRequestEndDoesNothingWhenSpanHasNoRequestContext() { @Test @SuppressWarnings("unchecked") void processRequestEndInvokesCallbackAndSkipsAsmTagsWhenNotManuallyKept() { + LambdaAppSecHandler.setCurrentTriggerType(LambdaTriggerType.API_GATEWAY_V1_REST); AppSecContext mockAppSecContext = mock(AppSecContext.class); when(mockAppSecContext.isManuallyKept()).thenReturn(false); TraceSegment mockTraceSegment = mock(TraceSegment.class); @@ -1097,6 +1085,7 @@ void processRequestEndInvokesCallbackAndSkipsAsmTagsWhenNotManuallyKept() { @Test void processRequestEndHandlesNullRequestEndedCallbackGracefully() { + LambdaAppSecHandler.setCurrentTriggerType(LambdaTriggerType.API_GATEWAY_V1_REST); RequestContext mockRequestContext = mock(RequestContext.class); AgentSpan span = mock(AgentSpan.class); when(span.getRequestContext()).thenReturn(mockRequestContext); @@ -1115,6 +1104,7 @@ void processRequestEndHandlesNullRequestEndedCallbackGracefully() { @Test @SuppressWarnings("unchecked") void processRequestEndSetsAsmKeepTagWhenAppSecContextIsManuallyKept() { + LambdaAppSecHandler.setCurrentTriggerType(LambdaTriggerType.API_GATEWAY_V1_REST); AppSecContext manuallyKeptCtx = mock(AppSecContext.class); when(manuallyKeptCtx.isManuallyKept()).thenReturn(true); @@ -1145,6 +1135,75 @@ void processRequestEndSetsAsmKeepTagWhenAppSecContextIsManuallyKept() { verify(mockTraceSegment).setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); } + @Test + void processRequestEndSetsUnsupportedEventTypeMetricForNonHttpTrigger() { + LambdaAppSecHandler.setCurrentTriggerType(LambdaTriggerType.UNKNOWN); + AgentSpan span = mock(AgentSpan.class); + + LambdaAppSecHandler.processRequestEnd(span); + + verify(span).setMetric("_dd.appsec.unsupported_event_type", 1); + verifyNoMoreInteractions(span); + } + + @Test + void processRequestEndSetsNoUnsupportedEventTypeMetricWhenNoTriggerTypeWasRecorded() { + // AppSec was inactive at request start and enabled mid-invocation + AgentSpan span = mock(AgentSpan.class); + + LambdaAppSecHandler.processRequestEnd(span); + + verify(span, never()).setMetric(anyString(), anyInt()); + } + + @Test + void processRequestEndSetsNoUnsupportedEventTypeMetricWhenAppSecIsDisabled() { + ActiveSubsystems.APPSEC_ACTIVE = false; + LambdaAppSecHandler.setCurrentTriggerType(LambdaTriggerType.UNKNOWN); + AgentSpan span = mock(AgentSpan.class); + + LambdaAppSecHandler.processRequestEnd(span); + + verifyNoInteractions(span); + } + + @Test + @SuppressWarnings("unchecked") + void processRequestEndSetsNoUnsupportedEventTypeMetricForHttpTrigger() { + LambdaAppSecHandler.setCurrentTriggerType(LambdaTriggerType.API_GATEWAY_V1_REST); + RequestContext mockRequestContext = mock(RequestContext.class); + when(mockRequestContext.getTraceSegment()).thenReturn(mock(TraceSegment.class)); + AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(mockRequestContext); + + BiFunction> requestEndedCallback = + mock(BiFunction.class); + when(requestEndedCallback.apply(any(), any())).thenReturn(new Flow.ResultFlow<>(null)); + CallbackProvider mockCallbackProvider = mock(CallbackProvider.class); + when(mockCallbackProvider.getCallback(EVENTS.requestEnded())).thenReturn(requestEndedCallback); + AgentTracer.TracerAPI mockTracer = mock(AgentTracer.TracerAPI.class); + when(mockTracer.getCallbackProvider(RequestContextSlot.APPSEC)) + .thenReturn(mockCallbackProvider); + AgentTracer.forceRegister(mockTracer); + + LambdaAppSecHandler.processRequestEnd(span); + + verify(requestEndedCallback).apply(mockRequestContext, span); + verify(span, never()).setMetric(anyString(), anyInt()); + } + + @Test + void processRequestEndSetsUnsupportedEventTypeMetricAfterAnUnparseablePayload() { + ByteArrayInputStream event = createInputStream("{invalid json"); + assertNull(LambdaAppSecHandler.processRequestStart(event)); + + AgentSpan span = mock(AgentSpan.class); + LambdaAppSecHandler.processRequestEnd(span); + + verify(span).setMetric("_dd.appsec.unsupported_event_type", 1); + verifyNoMoreInteractions(span); + } + // ============================================================================ // mergeContexts Tests // ============================================================================ @@ -2236,16 +2295,6 @@ void omitsRouteTagForAlbEvent() { assertEquals("alb-agent", tags.get(Tags.HTTP_USER_AGENT)); } - @Test - void appliesNoHttpTagsForNonHttpEvent() { - setupMockCallbacks(new Callbacks()); - AgentSpanContext context = - LambdaAppSecHandler.processRequestStart( - createInputStream("{\"Records\": [{\"eventSource\": \"aws:sqs\", \"body\": \"hi\"}]}")); - - assertTrue(tagsOf(context).isEmpty()); - } - @Test void mergeContextsCopiesHttpTagsIntoExtensionContext() { TagContext appSecContext = new TagContext(); diff --git a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaEventParserTest.java b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaEventParserTest.java index 9afd9d9ffa4..7811ab23c97 100644 --- a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaEventParserTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaEventParserTest.java @@ -270,13 +270,11 @@ void queryParametersKeepEventOrder() { // ============================================================================ @Test - void nonHttpEventHasNoHostOrRoute() { - LambdaRequestData data = - parseEvent("{\"Records\": [{\"eventSource\": \"aws:sqs\", \"body\": \"hello\"}]}"); - - assertEquals(LambdaTriggerType.UNKNOWN, data.triggerType); - assertNull(data.host); - assertNull(data.route); + void nonHttpEventIsNotExtracted() { + // AppSec skips non-HTTP triggers entirely, so nothing is extracted from their payload + assertSame( + LambdaRequestData.EMPTY, + parseEvent("{\"Records\": [{\"eventSource\": \"aws:sqs\", \"body\": \"hello\"}]}")); } @Test