Skip to content

Add fallback for missing http.route in API Security #8987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add fallback for missing http.route in API Security #8987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,11 +773,11 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
private boolean maybeSampleForApiSecurity(
AppSecRequestContext ctx, IGSpanInfo spanInfo, Map<String, Object> tags) {
log.debug("Checking API Security for end of request handler on span: {}", spanInfo.getSpanId());
// API Security sampling requires http.route tag.
// API Security sampling requires http.route tag. If it is not present, we set empty string to
// avoid filtering all requests when http route is not implemented for some frameworks.
final Object route = tags.get(Tags.HTTP_ROUTE);
if (route != null) {
ctx.setRoute(route.toString());
}
String routeStr = route != null ? route.toString() : "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really make sense?, even though we include some requests we still don´t have the http.route to link the request. What does it mean from a security perspective? e.g.: if we compute the request/response schemas how we are going to link them to the actual path? (maybe it can be done/ it's done in the backed using the path or other strategies)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manuel-alvarez-alvarez These can later use endpoint inference in the backend (upcoming).

ctx.setRoute(routeStr);
return requestSampler.preSampleRequest(ctx);
}

Expand Down
18 changes: 18 additions & 0 deletions ...agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,24 @@ class GatewayBridgeSpecification extends DDSpecification {
0 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM)
}

void 'test api security sampling - No http route'() {
given:
AppSecRequestContext mockAppSecCtx = Mock(AppSecRequestContext)
RequestContext mockCtx = Stub(RequestContext) {
getData(RequestContextSlot.APPSEC) >> mockAppSecCtx
getTraceSegment() >> traceSegment
}
IGSpanInfo spanInfo = Mock(AgentSpan)
when:
def flow = requestEndedCB.apply(mockCtx, spanInfo)
then:
1 * mockAppSecCtx.transferCollectedEvents() >> []
1 * spanInfo.getTags() >> ['http.route': null]
1 * requestSampler.preSampleRequest(_) >> true
0 * traceSegment.setTagTop(Tags.ASM_KEEP, true)
0 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM)
}

void 'test api security sampling - trace excluded'() {
given:
AppSecRequestContext mockAppSecCtx = Mock(AppSecRequestContext)
Expand Down
Loading