Skip to content

Commit b9ea9c5

Browse files
IlyasShabiIlyasShabi
authored
[APPSEC] use http endpoint on api security sampling algorithm when route is not available (#7062)
Use http endpoint on api security sampling algorithm when route is not available
1 parent 1a5eae4 commit b9ea9c5

File tree

10 files changed

+340
-35
lines changed

10 files changed

+340
-35
lines changed

‎packages/dd-trace/src/appsec/api_security_sampler.js‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,36 @@ function isSampled (key) {
7070
return sampledRequests.has(key)
7171
}
7272

73+
function getRouteOrEndpoint (context, statusCode) {
74+
// First try to get the route from the context paths
75+
const route = context?.paths?.join('') || ''
76+
if (route) {
77+
return route
78+
}
79+
80+
// If route is not available, fallback to http.endpoint
81+
if (statusCode !== 404) {
82+
const endpoint = context?.span?.context()?._tags?.['http.endpoint']
83+
if (endpoint) {
84+
return endpoint
85+
}
86+
}
87+
88+
return ''
89+
}
90+
7391
function computeKey (req, res) {
74-
const route = web.getContext(req)?.paths?.join('') || ''
7592
const method = req.method
7693
const status = res.statusCode
7794

7895
if (!method || !status) {
7996
log.warn('[ASM] Unsupported groupkey for API security')
8097
return null
8198
}
99+
100+
const context = web.getContext(req)
101+
const route = getRouteOrEndpoint(context, status)
102+
82103
return method + route + status
83104
}
84105

‎packages/dd-trace/src/appsec/index.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,9 @@ function incomingHttpEndTranslator ({ req, res }) {
233233
persistent[addresses.HTTP_INCOMING_QUERY] = query
234234
}
235235

236+
// This hook runs before span finish, so ensure route/endpoint tags are available before API Security sampling runs.
237+
web.setRouteOrEndpointTag(req)
238+
236239
if (apiSecuritySampler.sampleRequest(req, res, true)) {
237240
persistent[addresses.WAF_CONTEXT_PROCESSOR] = { 'extract-schema': true }
238241
}

‎packages/dd-trace/src/appsec/reporter.js‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,11 @@ function reportWafConfigUpdate (product, rcConfigId, diagnostics, wafVersion) {
300300
}
301301
}
302302

303-
function reportMetrics (metrics, raspRule) {
304-
const store = storage('legacy').getStore()
305-
const rootSpan = store?.req && web.root(store.req)
303+
function reportMetrics (metrics, raspRule, req) {
304+
if (!req) {
305+
req = storage('legacy').getStore()?.req
306+
}
307+
const rootSpan = req && web.root(req)
306308

307309
if (!rootSpan) return
308310

@@ -311,9 +313,9 @@ function reportMetrics (metrics, raspRule) {
311313
}
312314

313315
if (raspRule) {
314-
updateRaspRequestsMetricTags(metrics, store.req, raspRule)
316+
updateRaspRequestsMetricTags(metrics, req, raspRule)
315317
} else {
316-
updateWafRequestsMetricTags(metrics, store.req)
318+
updateWafRequestsMetricTags(metrics, req)
317319
}
318320

319321
reportTruncationMetrics(rootSpan, metrics)
@@ -333,9 +335,11 @@ function reportTruncationMetrics (rootSpan, metrics) {
333335
}
334336
}
335337

336-
function reportAttack ({ events: attackData, actions }) {
337-
const store = storage('legacy').getStore()
338-
const req = store?.req
338+
function reportAttack ({ events: attackData, actions }, req) {
339+
if (!req) {
340+
req = storage('legacy').getStore()?.req
341+
}
342+
339343
const rootSpan = web.root(req)
340344
if (!rootSpan) return
341345

@@ -473,10 +477,13 @@ function isSchemaAttribute (attribute) {
473477
return attribute.startsWith('_dd.appsec.s.')
474478
}
475479

476-
function reportAttributes (attributes) {
480+
function reportAttributes (attributes, req) {
477481
if (!attributes) return
478482

479-
const req = storage('legacy').getStore()?.req
483+
if (!req) {
484+
req = storage('legacy').getStore()?.req
485+
}
486+
480487
const rootSpan = web.root(req)
481488

482489
if (!rootSpan) return

‎packages/dd-trace/src/appsec/waf/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ function run (data, req, raspRule) {
122122
}
123123

124124
const wafContext = waf.wafManager.getWAFContext(req)
125-
const result = wafContext.run(data, raspRule)
125+
const result = wafContext.run(data, raspRule, req)
126126

127127
if (result?.keep) {
128128
if (limiter.isAllowed()) {

‎packages/dd-trace/src/appsec/waf/waf_context_wrapper.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class WAFContextWrapper {
2222
this.cachedUserIdResults = new Map()
2323
}
2424

25-
run ({ persistent, ephemeral }, raspRule) {
25+
run ({ persistent, ephemeral }, raspRule, req) {
2626
if (this.ddwafContext.disposed) {
2727
log.warn('[ASM] Calling run on a disposed context')
2828
if (raspRule) {
@@ -141,10 +141,10 @@ class WAFContextWrapper {
141141
metrics.wafTimeout = result.timeout
142142

143143
if (ruleTriggered) {
144-
Reporter.reportAttack(result)
144+
Reporter.reportAttack(result, req)
145145
}
146146

147-
Reporter.reportAttributes(result.attributes)
147+
Reporter.reportAttributes(result.attributes, req)
148148

149149
return result
150150
} catch (err) {
@@ -156,7 +156,7 @@ class WAFContextWrapper {
156156
wafRunFinished.publish({ payload })
157157
}
158158

159-
Reporter.reportMetrics(metrics, raspRule)
159+
Reporter.reportMetrics(metrics, raspRule, req)
160160
}
161161
}
162162

‎packages/dd-trace/src/plugins/util/web.js‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,13 @@ const web = {
410410
},
411411
})
412412
},
413+
setRouteOrEndpointTag (req) {
414+
const context = contexts.get(req)
415+
416+
if (!context) return
417+
418+
applyRouteOrEndpointTag(context)
419+
},
413420
}
414421

415422
function addAllowHeaders (req, res, headers) {
@@ -481,18 +488,9 @@ function addRequestTags (context, spanType) {
481488
}
482489

483490
function addResponseTags (context) {
484-
const { req, res, paths, span, inferredProxySpan, config } = context
491+
const { req, res, inferredProxySpan, span } = context
485492

486-
const route = paths.join('')
487-
if (route) {
488-
// Use http.route from trusted framework instrumentation
489-
span.setTag(HTTP_ROUTE, route)
490-
} else if (config.resourceRenamingEnabled) {
491-
// Route is unavailable, compute http.endpoint instead
492-
const url = span.context()._tags[HTTP_URL]
493-
const endpoint = url ? calculateHttpEndpoint(url) : '/'
494-
span.setTag(HTTP_ENDPOINT, endpoint)
495-
}
493+
applyRouteOrEndpointTag(context)
496494

497495
span.addTags({
498496
[HTTP_STATUS_CODE]: res.statusCode,
@@ -504,6 +502,28 @@ function addResponseTags (context) {
504502
web.addStatusError(req, res.statusCode)
505503
}
506504

505+
function applyRouteOrEndpointTag (context) {
506+
const { paths, span, config } = context
507+
if (!span) return
508+
const tags = span.context()._tags
509+
const route = paths.join('')
510+
511+
if (route) {
512+
// Use http.route from trusted framework instrumentation.
513+
span.setTag(HTTP_ROUTE, route)
514+
return
515+
}
516+
517+
if (!config.resourceRenamingEnabled || tags[HTTP_ENDPOINT]) {
518+
return
519+
}
520+
521+
// Route is unavailable, compute http.endpoint once.
522+
const url = tags[HTTP_URL]
523+
const endpoint = url ? calculateHttpEndpoint(url) : '/'
524+
span.setTag(HTTP_ENDPOINT, endpoint)
525+
}
526+
507527
function addResourceTag (context) {
508528
const { req, span } = context
509529
const tags = span.context()._tags

‎packages/dd-trace/test/appsec/api_security_sampler.spec.js‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,4 +242,103 @@ describe('API Security Sampler', () => {
242242
assert.strictEqual(keepTraceStub.calledOnceWith(span, 'asm'), true)
243243
})
244244
})
245+
246+
describe('http.endpoint', () => {
247+
beforeEach(() => {
248+
apiSecuritySampler.configure({ appsec: { apiSecurity: { enabled: true, sampleDelay: 30 } } })
249+
})
250+
251+
it('should use http.endpoint when http.route is not available', () => {
252+
const spanWithEndpoint = {
253+
context: sinon.stub().returns({
254+
_sampling: { priority: AUTO_KEEP },
255+
_tags: { 'http.endpoint': '/api/users' },
256+
}),
257+
}
258+
webStub.root.returns(spanWithEndpoint)
259+
webStub.getContext.returns({ paths: [], span: spanWithEndpoint })
260+
261+
const key = apiSecuritySampler.computeKey(req, res)
262+
assert.equal(key, 'GET/api/users200')
263+
})
264+
265+
it('should not use http.endpoint for 404 status codes', () => {
266+
const res404 = { statusCode: 404 }
267+
const spanWithEndpoint = {
268+
context: sinon.stub().returns({
269+
_sampling: { priority: AUTO_KEEP },
270+
_tags: { 'http.endpoint': '/api/users' },
271+
}),
272+
}
273+
webStub.root.returns(spanWithEndpoint)
274+
webStub.getContext.returns({ paths: [], span: spanWithEndpoint })
275+
276+
const key = apiSecuritySampler.computeKey(req, res404)
277+
assert.equal(key, 'GET404')
278+
})
279+
280+
it('should prefer http.route over http.endpoint when both are available', () => {
281+
const spanWithBoth = {
282+
context: sinon.stub().returns({
283+
_sampling: { priority: AUTO_KEEP },
284+
_tags: { 'http.endpoint': '/api/users' },
285+
}),
286+
}
287+
webStub.root.returns(spanWithBoth)
288+
webStub.getContext.returns({ paths: ['/users/:id'], span: spanWithBoth })
289+
290+
const key = apiSecuritySampler.computeKey(req, res)
291+
assert.equal(key, 'GET/users/:id200')
292+
})
293+
294+
it('should handle missing http.endpoint gracefully', () => {
295+
const spanWithoutEndpoint = {
296+
context: sinon.stub().returns({
297+
_sampling: { priority: AUTO_KEEP },
298+
_tags: {},
299+
}),
300+
}
301+
webStub.root.returns(spanWithoutEndpoint)
302+
webStub.getContext.returns({ paths: [], span: spanWithoutEndpoint })
303+
304+
const key = apiSecuritySampler.computeKey(req, res)
305+
assert.equal(key, 'GET200')
306+
})
307+
308+
it('should handle missing span gracefully', () => {
309+
webStub.getContext.returns({ paths: [], span: null })
310+
311+
const key = apiSecuritySampler.computeKey(req, res)
312+
assert.equal(key, 'GET200')
313+
})
314+
315+
it('should sample different endpoints separately', () => {
316+
const span1 = {
317+
context: sinon.stub().returns({
318+
_sampling: { priority: AUTO_KEEP },
319+
_tags: { 'http.endpoint': '/api/users' },
320+
}),
321+
}
322+
const span2 = {
323+
context: sinon.stub().returns({
324+
_sampling: { priority: AUTO_KEEP },
325+
_tags: { 'http.endpoint': '/api/products' },
326+
}),
327+
}
328+
329+
webStub.root.returns(span1)
330+
webStub.getContext.returns({ paths: [], span: span1 })
331+
assert.ok(apiSecuritySampler.sampleRequest(req, res, true))
332+
333+
webStub.root.returns(span2)
334+
webStub.getContext.returns({ paths: [], span: span2 })
335+
assert.ok(apiSecuritySampler.sampleRequest(req, res, true))
336+
337+
const key1 = apiSecuritySampler.computeKey(req, res)
338+
webStub.getContext.returns({ paths: [], span: span1 })
339+
const key2 = apiSecuritySampler.computeKey(req, res)
340+
341+
assert.notEqual(key1, key2)
342+
})
343+
})
245344
})

0 commit comments

Comments
 (0)