From e77d4aa5acfab11a8341c8b68ad2f9d8c85fffc1 Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 10 Mar 2025 13:59:20 +0100 Subject: [PATCH 01/14] waf requests telemetry metrics --- packages/dd-trace/src/appsec/blocking.js | 2 + packages/dd-trace/src/appsec/graphql.js | 4 +- packages/dd-trace/src/appsec/reporter.js | 5 +- .../dd-trace/src/appsec/telemetry/common.js | 8 +- .../dd-trace/src/appsec/telemetry/index.js | 16 ++ packages/dd-trace/src/appsec/telemetry/waf.js | 43 +++- .../dd-trace/test/appsec/blocking.spec.js | 15 +- packages/dd-trace/test/appsec/graphql.spec.js | 12 +- .../dd-trace/test/appsec/reporter.spec.js | 31 ++- .../dd-trace/test/appsec/resources/index.js | 21 ++ .../test/appsec/telemetry/rasp.spec.js | 4 + .../test/appsec/telemetry/waf.spec.js | 66 +++++-- .../appsec/waf-metrics.integration.spec.js | 185 ++++++++++++++++++ 13 files changed, 377 insertions(+), 35 deletions(-) create mode 100644 packages/dd-trace/test/appsec/resources/index.js create mode 100644 packages/dd-trace/test/appsec/waf-metrics.integration.spec.js diff --git a/packages/dd-trace/src/appsec/blocking.js b/packages/dd-trace/src/appsec/blocking.js index 5b3dd290c00..5e0090838d6 100644 --- a/packages/dd-trace/src/appsec/blocking.js +++ b/packages/dd-trace/src/appsec/blocking.js @@ -2,6 +2,7 @@ const log = require('../log') const blockedTemplates = require('./blocked_templates') +const { updateWafBlockFailureMetric } = require('./telemetry') const detectedSpecificEndpoints = {} @@ -128,6 +129,7 @@ function block (req, res, rootSpan, abortController, actionParameters = defaultB rootSpan?.setTag('_dd.appsec.block.failed', 1) log.error('[ASM] Blocking error', err) + updateWafBlockFailureMetric(req) return false } } diff --git a/packages/dd-trace/src/appsec/graphql.js b/packages/dd-trace/src/appsec/graphql.js index 2ff265e6282..bda5bb2bea9 100644 --- a/packages/dd-trace/src/appsec/graphql.js +++ b/packages/dd-trace/src/appsec/graphql.js @@ -17,6 +17,7 @@ const { apolloChannel, apolloServerCoreChannel } = require('./channels') +const { updateWafBlockFailureMetric } = require('./telemetry') const graphqlRequestData = new WeakMap() @@ -106,8 +107,9 @@ function beforeWriteApolloGraphqlResponse ({ abortController, abortData }) { abortController?.abort() } catch (err) { rootSpan.setTag('_dd.appsec.block.failed', 1) - log.error('[ASM] Blocking error', err) + + updateWafBlockFailureMetric(req) } } diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 9a0b7467c37..4e667cc4559 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -10,7 +10,8 @@ const { updateRaspRequestsMetricTags, incrementWafUpdatesMetric, incrementWafRequestsMetric, - getRequestMetrics + getRequestMetrics, + updateWafRateLimitedMetric } = require('./telemetry') const zlib = require('zlib') const { keepTrace } = require('../priority_sampler') @@ -147,6 +148,7 @@ function reportAttack (attackData) { if (limiter.isAllowed()) { keepTrace(rootSpan, ASM) + updateWafRateLimitedMetric(req) } // TODO: maybe add this to format.js later (to take decision as late as possible) @@ -202,6 +204,7 @@ function finishRequest (req, res) { rootSpan.addTags(Object.fromEntries(metricsQueue)) keepTrace(rootSpan, ASM) + updateWafRateLimitedMetric(req) metricsQueue.clear() } diff --git a/packages/dd-trace/src/appsec/telemetry/common.js b/packages/dd-trace/src/appsec/telemetry/common.js index 2b5b93801c3..ace08e0acd4 100644 --- a/packages/dd-trace/src/appsec/telemetry/common.js +++ b/packages/dd-trace/src/appsec/telemetry/common.js @@ -3,11 +3,15 @@ const DD_TELEMETRY_REQUEST_METRICS = Symbol('_dd.appsec.telemetry.request.metrics') const tags = { + BLOCK_FAILURE: 'block_failure', + EVENT_RULES_VERSION: 'event_rules_version', + INPUT_TRUNCATED: 'input_truncated', REQUEST_BLOCKED: 'request_blocked', + RATE_LIMITED: 'rate_limited', RULE_TRIGGERED: 'rule_triggered', + WAF_ERROR: 'waf_error', WAF_TIMEOUT: 'waf_timeout', - WAF_VERSION: 'waf_version', - EVENT_RULES_VERSION: 'event_rules_version' + WAF_VERSION: 'waf_version' } function getVersionsTags (wafVersion, rulesVersion) { diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index 1a0141c2068..ae2c9687e99 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -74,6 +74,20 @@ function updateWafRequestsMetricTags (metrics, req) { return trackWafMetrics(store, metrics) } +function updateWafRateLimitedMetric (req) { + if (!enabled) return + + const store = getStore(req) + trackWafMetrics(store, { rateLimited: true }) +} + +function updateWafBlockFailureMetric (req) { + if (!enabled) return + + const store = getStore(req) + trackWafMetrics(store, { blockFailed: true }) +} + function incrementWafInitMetric (wafVersion, rulesVersion) { if (!enabled) return @@ -125,6 +139,8 @@ module.exports = { incrementWafRequestsMetric, incrementMissingUserLoginMetric, incrementMissingUserIdMetric, + updateWafRateLimitedMetric, + updateWafBlockFailureMetric, getRequestMetrics } diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index 31b4ecafec6..56fc21ff1d3 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -7,6 +7,12 @@ const appsecMetrics = telemetryMetrics.manager.namespace('appsec') const DD_TELEMETRY_WAF_RESULT_TAGS = Symbol('_dd.appsec.telemetry.waf.result.tags') +const TRUNCATION_FLAGS = { + LONG_STRING: 1, + LARGE_CONTAINER: 2, + DEEP_CONTAINER: 4 +} + function addWafRequestMetrics (store, { duration, durationExt, wafTimeout, errorCode }) { store[DD_TELEMETRY_REQUEST_METRICS].duration += duration || 0 store[DD_TELEMETRY_REQUEST_METRICS].durationExt += durationExt || 0 @@ -44,17 +50,32 @@ function trackWafMetrics (store, metrics) { const metricTags = getOrCreateMetricTags(store, versionsTags) - const { blockTriggered, ruleTriggered, wafTimeout } = metrics + if (metrics.blockFailed) { + metricTags[tags.BLOCK_FAILURE] = true + } - if (blockTriggered) { + if (metrics.blockTriggered) { metricTags[tags.REQUEST_BLOCKED] = true } - if (ruleTriggered) { + if (metrics.errorCode) { + metricTags[tags.WAF_ERROR] = true + } + + if (metrics.rateLimited) { + metricTags[tags.RATE_LIMITED] = true + } + + if (metrics.ruleTriggered) { metricTags[tags.RULE_TRIGGERED] = true } - if (wafTimeout) { + const truncationReason = getTruncationReason(metrics) + if (truncationReason > 0) { + metricTags[tags.INPUT_TRUNCATED] = true + } + + if (metrics.wafTimeout) { metricTags[tags.WAF_TIMEOUT] = true } @@ -66,8 +87,12 @@ function getOrCreateMetricTags (store, versionsTags) { if (!metricTags) { metricTags = { + [tags.BLOCK_FAILURE]: false, + [tags.INPUT_TRUNCATED]: false, [tags.REQUEST_BLOCKED]: false, + [tags.RATE_LIMITED]: false, [tags.RULE_TRIGGERED]: false, + [tags.WAF_ERROR]: false, [tags.WAF_TIMEOUT]: false, ...versionsTags @@ -98,6 +123,16 @@ function incrementWafRequests (store) { } } +function getTruncationReason ({ maxTruncatedString, maxTruncatedContainerSize, maxTruncatedContainerDepth }) { + let reason = 0 + + if (maxTruncatedString) reason |= TRUNCATION_FLAGS.LONG_STRING + if (maxTruncatedContainerSize) reason |= TRUNCATION_FLAGS.LARGE_CONTAINER + if (maxTruncatedContainerDepth) reason |= TRUNCATION_FLAGS.DEEP_CONTAINER + + return reason +} + module.exports = { addWafRequestMetrics, trackWafMetrics, diff --git a/packages/dd-trace/test/appsec/blocking.spec.js b/packages/dd-trace/test/appsec/blocking.spec.js index 692ef58b6ef..03905de5a14 100644 --- a/packages/dd-trace/test/appsec/blocking.spec.js +++ b/packages/dd-trace/test/appsec/blocking.spec.js @@ -13,7 +13,7 @@ describe('blocking', () => { } } - let log + let log, telemetry let block, setTemplates let req, res, rootSpan @@ -22,9 +22,14 @@ describe('blocking', () => { warn: sinon.stub() } + telemetry = { + updateWafBlockFailureMetric: sinon.stub() + } + const blocking = proxyquire('../src/appsec/blocking', { '../log': log, - './blocked_templates': defaultBlockedTemplate + './blocked_templates': defaultBlockedTemplate, + './telemetry': telemetry }) block = blocking.block @@ -66,6 +71,7 @@ describe('blocking', () => { expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1) expect(res.setHeader).to.not.have.been.called expect(res.constructor.prototype.end).to.not.have.been.called + expect(telemetry.updateWafBlockFailureMetric).to.be.calledOnceWithExactly(req) }) it('should send blocking response with html type if present in the headers', () => { @@ -79,6 +85,7 @@ describe('blocking', () => { 'Content-Length': 12 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('htmlBodyéé') + expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called }) it('should send blocking response with json type if present in the headers in priority', () => { @@ -92,6 +99,7 @@ describe('blocking', () => { 'Content-Length': 8 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') + expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called }) it('should send blocking response with json type if neither html or json is present in the headers', () => { @@ -104,6 +112,7 @@ describe('blocking', () => { 'Content-Length': 8 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') + expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called }) it('should send blocking response and call abortController if passed in arguments', () => { @@ -118,6 +127,7 @@ describe('blocking', () => { }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') expect(abortController.signal.aborted).to.be.true + expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called }) it('should remove all headers before sending blocking response', () => { @@ -135,6 +145,7 @@ describe('blocking', () => { 'Content-Length': 8 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') + expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called }) }) diff --git a/packages/dd-trace/test/appsec/graphql.spec.js b/packages/dd-trace/test/appsec/graphql.spec.js index ab030e3082b..a31ca2af1d4 100644 --- a/packages/dd-trace/test/appsec/graphql.spec.js +++ b/packages/dd-trace/test/appsec/graphql.spec.js @@ -12,8 +12,7 @@ const { } = require('../../src/appsec/channels') describe('GraphQL', () => { - let graphql - let blocking + let graphql, blocking, telemetry beforeEach(() => { const getBlockingData = sinon.stub() @@ -29,8 +28,13 @@ describe('GraphQL', () => { statusCode: 403 }) + telemetry = { + updateWafBlockFailureMetric: sinon.stub() + } + graphql = proxyquire('../../src/appsec/graphql', { - './blocking': blocking + './blocking': blocking, + './telemetry': telemetry }) }) @@ -234,6 +238,7 @@ describe('GraphQL', () => { expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters) expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('appsec.blocked', 'true') + expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called }) it('Should catch error when block fails', () => { @@ -263,6 +268,7 @@ describe('GraphQL', () => { expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters) expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1) + expect(telemetry.updateWafBlockFailureMetric).to.be.calledOnceWithExactly(req) }) }) }) diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index 271a81e2725..b199365fb33 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -38,6 +38,7 @@ describe('reporter', () => { updateRaspRequestsMetricTags: sinon.stub(), incrementWafUpdatesMetric: sinon.stub(), incrementWafRequestsMetric: sinon.stub(), + updateWafRateLimitedMetric: sinon.stub(), getRequestMetrics: sinon.stub() } @@ -287,6 +288,7 @@ describe('reporter', () => { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) }) it('should add tags to request span', () => { @@ -301,30 +303,35 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) }) it('should not add manual.keep when rate limit is reached', (done) => { const addTags = span.addTags - const params = {} - expect(Reporter.reportAttack('', params)).to.not.be.false - expect(Reporter.reportAttack('', params)).to.not.be.false - expect(Reporter.reportAttack('', params)).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(3) + expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(3) Reporter.setRateLimit(1) - expect(Reporter.reportAttack('', params)).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) - expect(Reporter.reportAttack('', params)).to.not.be.false + expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(4) + + expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) + expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(4) setTimeout(() => { - expect(Reporter.reportAttack('', params)).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(5) + expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(5) done() }, 1020) }) @@ -332,7 +339,7 @@ describe('reporter', () => { it('should not overwrite origin tag', () => { span.context()._tags = { '_dd.origin': 'tracer' } - const result = Reporter.reportAttack('[]', {}) + const result = Reporter.reportAttack('[]') expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -342,6 +349,7 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) }) it('should merge attacks json', () => { @@ -358,9 +366,10 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) }) - it('should call standalone sample', () => { + it('should call standalone sample and updateWafRateLimitedMetric', () => { span.context()._tags = { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' } const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]') @@ -375,6 +384,7 @@ describe('reporter', () => { }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) }) }) @@ -747,7 +757,7 @@ describe('reporter', () => { expect(span.setTag).to.have.been.calledWithExactly('_dd.appsec.rasp.error', -1) }) - it('should keep span if there are metrics', () => { + it('should keep span and call updateWafRateLimitedMetric if there are metrics', () => { const req = {} Reporter.metricsQueue.set('a', 1) @@ -756,6 +766,7 @@ describe('reporter', () => { Reporter.finishRequest(req, wafContext, {}) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) }) }) }) diff --git a/packages/dd-trace/test/appsec/resources/index.js b/packages/dd-trace/test/appsec/resources/index.js new file mode 100644 index 00000000000..575e4724ec7 --- /dev/null +++ b/packages/dd-trace/test/appsec/resources/index.js @@ -0,0 +1,21 @@ +'use strict' + +const tracer = require('dd-trace') +tracer.init({ + flushInterval: 1 +}) + +const express = require('express') +const body = require('body-parser') + +const app = express() +app.use(body.json()) +const port = process.env.APP_PORT || 3000 + +app.post('/', async (req, res) => { + res.end('OK') +}) + +app.listen(port, () => { + process.send({ port }) +}) diff --git a/packages/dd-trace/test/appsec/telemetry/rasp.spec.js b/packages/dd-trace/test/appsec/telemetry/rasp.spec.js index 3640fb3e9d4..1cf0b86056a 100644 --- a/packages/dd-trace/test/appsec/telemetry/rasp.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/rasp.spec.js @@ -137,8 +137,12 @@ describe('Appsec Rasp Telemetry metrics', () => { appsecTelemetry.incrementWafRequestsMetric(req) expect(count).to.have.been.calledWithExactly('waf.requests', { + block_failure: false, + input_truncated: false, request_blocked: false, + rate_limited: false, rule_triggered: false, + waf_error: false, waf_timeout: false, waf_version: wafVersion, event_rules_version: rulesVersion diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index 3e16714c234..dc0a79ee75b 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -53,11 +53,15 @@ describe('Appsec Waf Telemetry metrics', () => { const result = appsecTelemetry.updateWafRequestsMetricTags(metrics, req) expect(result).to.be.deep.eq({ - waf_version: wafVersion, + block_failure: false, event_rules_version: rulesVersion, + input_truncated: false, + rate_limited: false, request_blocked: false, rule_triggered: false, - waf_timeout: false + waf_error: false, + waf_timeout: false, + waf_version: wafVersion }) }) @@ -66,15 +70,22 @@ describe('Appsec Waf Telemetry metrics', () => { blockTriggered: true, ruleTriggered: true, wafTimeout: true, + rateLimited: true, + errorCode: -1, + maxTruncatedString: 5000, ...metrics }, req) expect(result).to.be.deep.eq({ - waf_version: wafVersion, + block_failure: false, event_rules_version: rulesVersion, + input_truncated: true, + rate_limited: true, request_blocked: true, rule_triggered: true, - waf_timeout: true + waf_error: true, + waf_timeout: true, + waf_version: wafVersion }) }) @@ -83,17 +94,22 @@ describe('Appsec Waf Telemetry metrics', () => { const result2 = appsecTelemetry.updateWafRequestsMetricTags({ ruleTriggered: true, + rateLimited: true, ...metrics }, req) expect(result).to.be.eq(result2) expect(result).to.be.deep.eq({ - waf_version: wafVersion, + block_failure: false, event_rules_version: rulesVersion, + input_truncated: false, + rate_limited: true, request_blocked: false, rule_triggered: true, - waf_timeout: false + waf_error: false, + waf_timeout: false, + waf_version: wafVersion }) }) @@ -102,6 +118,7 @@ describe('Appsec Waf Telemetry metrics', () => { blockTriggered: true, ruleTriggered: true, wafTimeout: true, + rateLimited: true, ...metrics }, req) @@ -110,17 +127,22 @@ describe('Appsec Waf Telemetry metrics', () => { blockTriggered: false, ruleTriggered: false, wafTimeout: false, + rateLimited: false, ...metrics }, req2) expect(result).to.be.not.eq(result2) expect(result).to.be.deep.eq({ - waf_version: wafVersion, + block_failure: false, event_rules_version: rulesVersion, + input_truncated: false, + rate_limited: true, request_blocked: true, rule_triggered: true, - waf_timeout: true + waf_error: false, + waf_timeout: true, + waf_version: wafVersion }) }) @@ -236,9 +258,13 @@ describe('Appsec Waf Telemetry metrics', () => { describe('incWafRequestsMetric', () => { it('should increment waf.requests metric', () => { appsecTelemetry.updateWafRequestsMetricTags({ - blockTriggered: false, - ruleTriggered: false, + blockTriggered: true, + blockFailed: true, + ruleTriggered: true, wafTimeout: true, + errorCode: -3, + rateLimited: true, + maxTruncatedString: 5000, wafVersion, rulesVersion }, req) @@ -246,12 +272,28 @@ describe('Appsec Waf Telemetry metrics', () => { appsecTelemetry.incrementWafRequestsMetric(req) expect(count).to.have.been.calledOnceWithExactly('waf.requests', { - request_blocked: false, - rule_triggered: false, + request_blocked: true, + block_failure: true, + rule_triggered: true, waf_timeout: true, + waf_error: true, + rate_limited: true, + input_truncated: true, waf_version: wafVersion, event_rules_version: rulesVersion }) + expect(count).to.have.been.calledOnce + expect(count.firstCall.args[1]).to.deep.equal({ + block_failure: true, + input_truncated: true, + request_blocked: true, + rate_limited: true, + rule_triggered: true, + waf_error: true, + waf_timeout: true, + waf_version: '0.0.1', + event_rules_version: '0.0.2' + }) }) it('should not fail if req has no previous tag', () => { diff --git a/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js new file mode 100644 index 00000000000..9da51614fa2 --- /dev/null +++ b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js @@ -0,0 +1,185 @@ +'use strict' + +const { createSandbox, FakeAgent, spawnProc } = require('../../../../integration-tests/helpers') +const getPort = require('get-port') +const path = require('path') +const Axios = require('axios') +const { assert } = require('chai') + +describe('WAF error metrics', () => { + let axios, sandbox, cwd, appPort, appFile, agent, proc + + before(async function () { + this.timeout(process.platform === 'win32' ? 90000 : 30000) + + sandbox = await createSandbox( + ['express'], + false, + [path.join(__dirname, 'resources')] + ) + + appPort = await getPort() + cwd = sandbox.folder + appFile = path.join(cwd, 'resources', 'index.js') + + axios = Axios.create({ + baseURL: `http://localhost:${appPort}` + }) + }) + + after(async function () { + this.timeout(60000) + await sandbox.remove() + }) + + beforeEach(async () => { + agent = await new FakeAgent().start() + proc = await spawnProc(appFile, { + cwd, + env: { + DD_TRACE_AGENT_PORT: agent.port, + APP_PORT: appPort, + DD_APPSEC_ENABLED: 'true', + DD_TELEMETRY_HEARTBEAT_INTERVAL: 1, + DD_APPSEC_WAF_TIMEOUT: 0.1 + } + }) + }) + + afterEach(async () => { + proc.kill() + await agent.stop() + }) + + it('should report waf error metrics', async () => { + let appsecTelemetryMetricsReceived = false + + const body = { + name: 'hey' + } + + await axios.post('/', body) + + const checkMessages = agent.assertMessageReceived(({ payload }) => { + assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) + assert.strictEqual(payload[0][0].metrics['_dd.appsec.waf.error'], -127) + }) + + const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { + const namespace = payload.payload.namespace + + if (namespace === 'appsec') { + appsecTelemetryMetricsReceived = true + const series = payload.payload.series + const wafRequests = series.find(s => s.metric === 'waf.requests') + + assert.exists(wafRequests, 'Waf requests serie should exist') + assert.strictEqual(wafRequests.type, 'count') + assert.include(wafRequests.tags, 'waf_error:true') + assert.include(wafRequests.tags, 'rate_limited:true') + } + }, 30_000, 'generate-metrics', 2) + + return Promise.all([checkMessages, checkTelemetryMetrics]).then(() => { + assert.equal(appsecTelemetryMetricsReceived, true) + + return true + }) + }) +}) + +describe('WAF timeout metrics', () => { + let axios, sandbox, cwd, appPort, appFile, agent, proc + + before(async function () { + this.timeout(process.platform === 'win32' ? 90000 : 30000) + + sandbox = await createSandbox( + ['express'], + false, + [path.join(__dirname, 'resources')] + ) + + appPort = await getPort() + cwd = sandbox.folder + appFile = path.join(cwd, 'resources', 'index.js') + + axios = Axios.create({ + baseURL: `http://localhost:${appPort}` + }) + }) + + after(async function () { + this.timeout(60000) + await sandbox.remove() + }) + + beforeEach(async () => { + agent = await new FakeAgent().start() + proc = await spawnProc(appFile, { + cwd, + env: { + DD_TRACE_AGENT_PORT: agent.port, + APP_PORT: appPort, + DD_APPSEC_ENABLED: 'true', + DD_TELEMETRY_HEARTBEAT_INTERVAL: 1, + DD_APPSEC_WAF_TIMEOUT: 1 + } + }) + }) + + afterEach(async () => { + proc.kill() + await agent.stop() + }) + + it('should report waf timeout metrics', async () => { + let appsecTelemetryMetricsReceived = false + + const longValue = 'testattack'.repeat(500) + const largeObject = {} + for (let i = 0; i < 300; ++i) { + largeObject[`key${i}`] = `value${i}` + } + const deepObject = createNestedObject(25, { value: 'a' }) + const complexPayload = { + deepObject, + longValue, + largeObject + } + + await axios.post('/', { complexPayload }) + + const checkMessages = agent.assertMessageReceived(({ payload }) => { + assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) + assert.isTrue(payload[0][0].metrics['_dd.appsec.waf.timeouts'] > 0) + }) + + const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { + const namespace = payload.payload.namespace + + if (namespace === 'appsec') { + appsecTelemetryMetricsReceived = true + const series = payload.payload.series + const wafRequests = series.find(s => s.metric === 'waf.requests') + + assert.exists(wafRequests, 'Waf requests serie should exist') + assert.strictEqual(wafRequests.type, 'count') + assert.include(wafRequests.tags, 'waf_timeout:true') + } + }, 30_000, 'generate-metrics', 2) + + return Promise.all([checkMessages, checkTelemetryMetrics]).then(() => { + assert.equal(appsecTelemetryMetricsReceived, true) + + return true + }) + }) +}) + +const createNestedObject = (n, obj) => { + if (n > 0) { + return { a: createNestedObject(n - 1, obj) } + } + return obj +} From fd83b58efb7f88f75f910cc472b04440565eb94a Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 10 Mar 2025 15:00:58 +0100 Subject: [PATCH 02/14] fix undefined versions --- packages/dd-trace/src/appsec/telemetry/waf.js | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index 56fc21ff1d3..1c75d862120 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -48,18 +48,23 @@ function trackWafMetrics (store, metrics) { trackWafDurations(metrics, versionsTags) - const metricTags = getOrCreateMetricTags(store, versionsTags) + const metricTags = getOrCreateMetricTags(store) if (metrics.blockFailed) { metricTags[tags.BLOCK_FAILURE] = true } - if (metrics.blockTriggered) { - metricTags[tags.REQUEST_BLOCKED] = true + if (versionsTags[tags.EVENT_RULES_VERSION]) { + metricTags[tags.EVENT_RULES_VERSION] = versionsTags[tags.EVENT_RULES_VERSION] } - if (metrics.errorCode) { - metricTags[tags.WAF_ERROR] = true + const truncationReason = getTruncationReason(metrics) + if (truncationReason > 0) { + metricTags[tags.INPUT_TRUNCATED] = true + } + + if (metrics.blockTriggered) { + metricTags[tags.REQUEST_BLOCKED] = true } if (metrics.rateLimited) { @@ -70,32 +75,35 @@ function trackWafMetrics (store, metrics) { metricTags[tags.RULE_TRIGGERED] = true } - const truncationReason = getTruncationReason(metrics) - if (truncationReason > 0) { - metricTags[tags.INPUT_TRUNCATED] = true + if (metrics.errorCode) { + metricTags[tags.WAF_ERROR] = true } if (metrics.wafTimeout) { metricTags[tags.WAF_TIMEOUT] = true } + if (versionsTags[tags.WAF_VERSION]) { + metricTags[tags.WAF_VERSION] = versionsTags[tags.WAF_VERSION] + } + return metricTags } -function getOrCreateMetricTags (store, versionsTags) { +function getOrCreateMetricTags (store) { let metricTags = store[DD_TELEMETRY_WAF_RESULT_TAGS] if (!metricTags) { metricTags = { [tags.BLOCK_FAILURE]: false, + [tags.EVENT_RULES_VERSION]: null, [tags.INPUT_TRUNCATED]: false, [tags.REQUEST_BLOCKED]: false, [tags.RATE_LIMITED]: false, [tags.RULE_TRIGGERED]: false, [tags.WAF_ERROR]: false, [tags.WAF_TIMEOUT]: false, - - ...versionsTags + [tags.WAF_VERSION]: null } store[DD_TELEMETRY_WAF_RESULT_TAGS] = metricTags } From ab489fa047f184b6573dc6cff484187ea1407b9c Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 10 Mar 2025 15:33:24 +0100 Subject: [PATCH 03/14] add more tests --- .../test/appsec/telemetry/waf.spec.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index dc0a79ee75b..4e05b6ed3b1 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -302,6 +302,22 @@ describe('Appsec Waf Telemetry metrics', () => { expect(count).to.not.have.been.called }) }) + + describe('updateWafRateLimitedMetric', () => { + it('should set rate_limited to true on the request tags', () => { + appsecTelemetry.updateWafRateLimitedMetric(req) + const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) + expect(result.rate_limited).to.be.true + }) + }) + + describe('updateWafBlockFailureMetric', () => { + it('should set block_failure to true on the request tags', () => { + appsecTelemetry.updateWafBlockFailureMetric(req) + const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) + expect(result.block_failure).to.be.true + }) + }) }) describe('if disabled', () => { @@ -329,6 +345,18 @@ describe('Appsec Waf Telemetry metrics', () => { expect(inc).to.not.have.been.called }) + it('should not set rate_limited if telemetry is disabled', () => { + appsecTelemetry.updateWafRateLimitedMetric(req) + const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) + expect(result).to.be.undefined + }) + + it('should not set block_failure if telemetry is disabled', () => { + appsecTelemetry.updateWafBlockFailureMetric(req) + const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) + expect(result).to.be.undefined + }) + describe('updateWafRequestMetricTags', () => { it('should sum waf.duration and waf.durationExt request metrics', () => { appsecTelemetry.enable({ From bc2a36ec438342a377108e7c8f3ee95b9645a7de Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 13 Mar 2025 12:52:35 +0100 Subject: [PATCH 04/14] fix tags order --- packages/dd-trace/src/appsec/telemetry/common.js | 2 +- packages/dd-trace/src/appsec/telemetry/waf.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/src/appsec/telemetry/common.js b/packages/dd-trace/src/appsec/telemetry/common.js index ace08e0acd4..d91cd9988b7 100644 --- a/packages/dd-trace/src/appsec/telemetry/common.js +++ b/packages/dd-trace/src/appsec/telemetry/common.js @@ -6,8 +6,8 @@ const tags = { BLOCK_FAILURE: 'block_failure', EVENT_RULES_VERSION: 'event_rules_version', INPUT_TRUNCATED: 'input_truncated', - REQUEST_BLOCKED: 'request_blocked', RATE_LIMITED: 'rate_limited', + REQUEST_BLOCKED: 'request_blocked', RULE_TRIGGERED: 'rule_triggered', WAF_ERROR: 'waf_error', WAF_TIMEOUT: 'waf_timeout', diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index 1c75d862120..8acaee8f890 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -98,8 +98,8 @@ function getOrCreateMetricTags (store) { [tags.BLOCK_FAILURE]: false, [tags.EVENT_RULES_VERSION]: null, [tags.INPUT_TRUNCATED]: false, - [tags.REQUEST_BLOCKED]: false, [tags.RATE_LIMITED]: false, + [tags.REQUEST_BLOCKED]: false, [tags.RULE_TRIGGERED]: false, [tags.WAF_ERROR]: false, [tags.WAF_TIMEOUT]: false, From 7924c421601f351dd64ede6cb89de309f490f0ea Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 19 Mar 2025 10:25:51 +0100 Subject: [PATCH 05/14] update waf metrics integration test --- .../appsec/waf-metrics.integration.spec.js | 400 ++++++++---------- 1 file changed, 176 insertions(+), 224 deletions(-) diff --git a/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js index 07e09d486e2..991f0589905 100644 --- a/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js +++ b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js @@ -6,8 +6,8 @@ const path = require('path') const Axios = require('axios') const { assert } = require('chai') -describe('WAF error metrics', () => { - let axios, sandbox, cwd, appPort, appFile, agent, proc +describe('WAF Metrics', () => { + let axios, sandbox, cwd, appPort, appFile before(async function () { this.timeout(process.platform === 'win32' ? 90000 : 30000) @@ -32,266 +32,218 @@ describe('WAF error metrics', () => { await sandbox.remove() }) - beforeEach(async () => { - agent = await new FakeAgent().start() - proc = await spawnProc(appFile, { - cwd, - env: { - DD_TRACE_AGENT_PORT: agent.port, - APP_PORT: appPort, - DD_APPSEC_ENABLED: 'true', - DD_TELEMETRY_HEARTBEAT_INTERVAL: 1, - DD_APPSEC_WAF_TIMEOUT: 0.1 - } + describe('WAF error metrics', () => { + let agent, proc + + beforeEach(async () => { + agent = await new FakeAgent().start() + proc = await spawnProc(appFile, { + cwd, + env: { + DD_TRACE_AGENT_PORT: agent.port, + APP_PORT: appPort, + DD_APPSEC_ENABLED: 'true', + DD_TELEMETRY_HEARTBEAT_INTERVAL: 1, + DD_APPSEC_WAF_TIMEOUT: 0.1 + } + }) }) - }) - - afterEach(async () => { - proc.kill() - await agent.stop() - }) - - it('should report waf error metrics', async () => { - let appsecTelemetryMetricsReceived = false - const body = { - name: 'hey' - } - - await axios.post('/', body) - - const checkMessages = agent.assertMessageReceived(({ payload }) => { - assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) - assert.strictEqual(payload[0][0].metrics['_dd.appsec.waf.error'], -127) + afterEach(async () => { + proc.kill() + await agent.stop() }) - const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { - const namespace = payload.payload.namespace - - if (namespace === 'appsec') { - appsecTelemetryMetricsReceived = true - const series = payload.payload.series - const wafRequests = series.find(s => s.metric === 'waf.requests') + it('should report waf error metrics', async () => { + let appsecTelemetryMetricsReceived = false - assert.exists(wafRequests, 'Waf requests serie should exist') - assert.strictEqual(wafRequests.type, 'count') - assert.include(wafRequests.tags, 'waf_error:true') - assert.include(wafRequests.tags, 'rate_limited:true') + const body = { + name: 'hey' } - }, 30_000, 'generate-metrics', 2) - return Promise.all([checkMessages, checkTelemetryMetrics]).then(() => { - assert.equal(appsecTelemetryMetricsReceived, true) + await axios.post('/', body) - return true - }) - }) -}) + const checkMessages = agent.assertMessageReceived(({ payload }) => { + assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) + assert.strictEqual(payload[0][0].metrics['_dd.appsec.waf.error'], -127) + }) -describe('WAF timeout metrics', () => { - let axios, sandbox, cwd, appPort, appFile, agent, proc + const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { + const namespace = payload.payload.namespace - before(async function () { - this.timeout(process.platform === 'win32' ? 90000 : 30000) + if (namespace === 'appsec') { + appsecTelemetryMetricsReceived = true + const series = payload.payload.series + const wafRequests = series.find(s => s.metric === 'waf.requests') - sandbox = await createSandbox( - ['express'], - false, - [path.join(__dirname, 'resources')] - ) + assert.exists(wafRequests, 'Waf requests serie should exist') + assert.strictEqual(wafRequests.type, 'count') + assert.include(wafRequests.tags, 'waf_error:true') + assert.include(wafRequests.tags, 'rate_limited:true') + } + }, 30_000, 'generate-metrics', 2) - appPort = await getPort() - cwd = sandbox.folder - appFile = path.join(cwd, 'resources', 'index.js') + return Promise.all([checkMessages, checkTelemetryMetrics]).then(() => { + assert.equal(appsecTelemetryMetricsReceived, true) - axios = Axios.create({ - baseURL: `http://localhost:${appPort}` + return true + }) }) }) - after(async function () { - this.timeout(60000) - await sandbox.remove() - }) - - beforeEach(async () => { - agent = await new FakeAgent().start() - proc = await spawnProc(appFile, { - cwd, - env: { - DD_TRACE_AGENT_PORT: agent.port, - APP_PORT: appPort, - DD_APPSEC_ENABLED: 'true', - DD_TELEMETRY_HEARTBEAT_INTERVAL: 1, - DD_APPSEC_WAF_TIMEOUT: 1 - } + describe('WAF timeout metrics', () => { + let agent, proc + + beforeEach(async () => { + agent = await new FakeAgent().start() + proc = await spawnProc(appFile, { + cwd, + env: { + DD_TRACE_AGENT_PORT: agent.port, + APP_PORT: appPort, + DD_APPSEC_ENABLED: 'true', + DD_TELEMETRY_HEARTBEAT_INTERVAL: 1, + DD_APPSEC_WAF_TIMEOUT: 1 + } + }) }) - }) - afterEach(async () => { - proc.kill() - await agent.stop() - }) - - it('should report waf timeout metrics', async () => { - let appsecTelemetryMetricsReceived = false - - const longValue = 'testattack'.repeat(500) - const largeObject = {} - for (let i = 0; i < 300; ++i) { - largeObject[`key${i}`] = `value${i}` - } - const deepObject = createNestedObject(25, { value: 'a' }) - const complexPayload = { - deepObject, - longValue, - largeObject - } - - await axios.post('/', { complexPayload }) - - const checkMessages = agent.assertMessageReceived(({ payload }) => { - assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) - assert.isTrue(payload[0][0].metrics['_dd.appsec.waf.timeouts'] > 0) + afterEach(async () => { + proc.kill() + await agent.stop() }) - const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { - const namespace = payload.payload.namespace + it('should report waf timeout metrics', async () => { + let appsecTelemetryMetricsReceived = false - if (namespace === 'appsec') { - appsecTelemetryMetricsReceived = true - const series = payload.payload.series - const wafRequests = series.find(s => s.metric === 'waf.requests') + const complexPayload = createComplexPayload() + await axios.post('/', { complexPayload }) - assert.exists(wafRequests, 'Waf requests serie should exist') - assert.strictEqual(wafRequests.type, 'count') - assert.include(wafRequests.tags, 'waf_timeout:true') - } - }, 30_000, 'generate-metrics', 2) - - return Promise.all([checkMessages, checkTelemetryMetrics]).then(() => { - assert.equal(appsecTelemetryMetricsReceived, true) - - return true - }) - }) -}) + const checkMessages = agent.assertMessageReceived(({ payload }) => { + assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) + assert.isTrue(payload[0][0].metrics['_dd.appsec.waf.timeouts'] > 0) + }) -describe('WAF truncation metrics', () => { - let axios, sandbox, cwd, appPort, appFile, agent, proc + const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { + const namespace = payload.payload.namespace - before(async function () { - this.timeout(process.platform === 'win32' ? 90000 : 30000) + if (namespace === 'appsec') { + appsecTelemetryMetricsReceived = true + const series = payload.payload.series + const wafRequests = series.find(s => s.metric === 'waf.requests') - sandbox = await createSandbox( - ['express'], - false, - [path.join(__dirname, 'resources')] - ) + assert.exists(wafRequests, 'Waf requests serie should exist') + assert.strictEqual(wafRequests.type, 'count') + assert.include(wafRequests.tags, 'waf_timeout:true') + } + }, 30_000, 'generate-metrics', 2) - appPort = await getPort() - cwd = sandbox.folder - appFile = path.join(cwd, 'resources', 'index.js') + return Promise.all([checkMessages, checkTelemetryMetrics]).then(() => { + assert.equal(appsecTelemetryMetricsReceived, true) - axios = Axios.create({ - baseURL: `http://localhost:${appPort}` + return true + }) }) }) - after(async function () { - this.timeout(60000) - await sandbox.remove() - }) - - beforeEach(async () => { - agent = await new FakeAgent().start() - proc = await spawnProc(appFile, { - cwd, - env: { - DD_TRACE_AGENT_PORT: agent.port, - APP_PORT: appPort, - DD_APPSEC_ENABLED: 'true', - DD_TELEMETRY_HEARTBEAT_INTERVAL: 1 - } + describe('WAF truncation metrics', () => { + let agent, proc + + beforeEach(async () => { + agent = await new FakeAgent().start() + proc = await spawnProc(appFile, { + cwd, + env: { + DD_TRACE_AGENT_PORT: agent.port, + APP_PORT: appPort, + DD_APPSEC_ENABLED: 'true', + DD_TELEMETRY_HEARTBEAT_INTERVAL: 1 + } + }) }) - }) - - afterEach(async () => { - proc.kill() - await agent.stop() - }) - it('should report tuncation metrics', async () => { - let appsecTelemetryMetricsReceived = false - let appsecTelemetryDistributionsReceived = false - - const longValue = 'testattack'.repeat(500) - const largeObject = {} - for (let i = 0; i < 300; ++i) { - largeObject[`key${i}`] = `value${i}` - } - const deepObject = createNestedObject(25, { value: 'a' }) - const complexPayload = { - deepObject, - longValue, - largeObject - } - - await axios.post('/', { complexPayload }) - - const checkMessages = agent.assertMessageReceived(({ payload }) => { - assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) - assert.strictEqual(payload[0][0].metrics['_dd.appsec.truncated.container_depth'], 20) - assert.strictEqual(payload[0][0].metrics['_dd.appsec.truncated.container_size'], 300) - assert.strictEqual(payload[0][0].metrics['_dd.appsec.truncated.string_length'], 5000) + afterEach(async () => { + proc.kill() + await agent.stop() }) - const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { - const namespace = payload.payload.namespace - - if (namespace === 'appsec') { - appsecTelemetryMetricsReceived = true - const series = payload.payload.series - const inputTruncated = series.find(s => s.metric === 'waf.input_truncated') - - assert.exists(inputTruncated, 'input truncated serie should exist') - assert.strictEqual(inputTruncated.type, 'count') - assert.include(inputTruncated.tags, 'truncation_reason:7') - - const wafRequests = series.find(s => s.metric === 'waf.requests') - assert.exists(wafRequests, 'waf requests serie should exist') - assert.include(wafRequests.tags, 'input_truncated:true') - } - }, 30_000, 'generate-metrics', 2) - - const checkTelemetryDistributions = agent.assertTelemetryReceived(({ payload }) => { - const namespace = payload.payload.namespace - - if (namespace === 'appsec') { - appsecTelemetryDistributionsReceived = true - const series = payload.payload.series - const wafDuration = series.find(s => s.metric === 'waf.duration') - const wafDurationExt = series.find(s => s.metric === 'waf.duration_ext') - const wafTuncated = series.filter(s => s.metric === 'waf.truncated_value_size') - - assert.exists(wafDuration, 'waf duration serie should exist') - assert.exists(wafDurationExt, 'waf duration ext serie should exist') - - assert.equal(wafTuncated.length, 3) - assert.include(wafTuncated[0].tags, 'truncation_reason:1') - assert.include(wafTuncated[1].tags, 'truncation_reason:2') - assert.include(wafTuncated[2].tags, 'truncation_reason:4') - } - }, 30_000, 'distributions', 1) - return Promise.all([checkMessages, checkTelemetryMetrics, checkTelemetryDistributions]).then(() => { - assert.equal(appsecTelemetryMetricsReceived, true) - assert.equal(appsecTelemetryDistributionsReceived, true) - - return true + it('should report truncation metrics', async () => { + let appsecTelemetryMetricsReceived = false + let appsecTelemetryDistributionsReceived = false + + const complexPayload = createComplexPayload() + await axios.post('/', { complexPayload }) + + const checkMessages = agent.assertMessageReceived(({ payload }) => { + assert.strictEqual(payload[0][0].metrics['_dd.appsec.enabled'], 1) + assert.strictEqual(payload[0][0].metrics['_dd.appsec.truncated.container_depth'], 20) + assert.strictEqual(payload[0][0].metrics['_dd.appsec.truncated.container_size'], 300) + assert.strictEqual(payload[0][0].metrics['_dd.appsec.truncated.string_length'], 5000) + }) + + const checkTelemetryMetrics = agent.assertTelemetryReceived(({ payload }) => { + const namespace = payload.payload.namespace + + if (namespace === 'appsec') { + appsecTelemetryMetricsReceived = true + const series = payload.payload.series + const inputTruncated = series.find(s => s.metric === 'waf.input_truncated') + + assert.exists(inputTruncated, 'input truncated serie should exist') + assert.strictEqual(inputTruncated.type, 'count') + assert.include(inputTruncated.tags, 'truncation_reason:7') + + const wafRequests = series.find(s => s.metric === 'waf.requests') + assert.exists(wafRequests, 'waf requests serie should exist') + assert.include(wafRequests.tags, 'input_truncated:true') + } + }, 30_000, 'generate-metrics', 2) + + const checkTelemetryDistributions = agent.assertTelemetryReceived(({ payload }) => { + const namespace = payload.payload.namespace + + if (namespace === 'appsec') { + appsecTelemetryDistributionsReceived = true + const series = payload.payload.series + const wafDuration = series.find(s => s.metric === 'waf.duration') + const wafDurationExt = series.find(s => s.metric === 'waf.duration_ext') + const wafTuncated = series.filter(s => s.metric === 'waf.truncated_value_size') + + assert.exists(wafDuration, 'waf duration serie should exist') + assert.exists(wafDurationExt, 'waf duration ext serie should exist') + + assert.equal(wafTuncated.length, 3) + assert.include(wafTuncated[0].tags, 'truncation_reason:1') + assert.include(wafTuncated[1].tags, 'truncation_reason:2') + assert.include(wafTuncated[2].tags, 'truncation_reason:4') + } + }, 30_000, 'distributions', 1) + + return Promise.all([checkMessages, checkTelemetryMetrics, checkTelemetryDistributions]).then(() => { + assert.equal(appsecTelemetryMetricsReceived, true) + assert.equal(appsecTelemetryDistributionsReceived, true) + + return true + }) }) }) }) +const createComplexPayload = () => { + const longValue = 'testattack'.repeat(500) + const largeObject = {} + for (let i = 0; i < 300; ++i) { + largeObject[`key${i}`] = `value${i}` + } + const deepObject = createNestedObject(25, { value: 'a' }) + + return { + deepObject, + longValue, + largeObject + } +} + const createNestedObject = (n, obj) => { if (n > 0) { return { a: createNestedObject(n - 1, obj) } From 482d1ce074bd7ebe98aa4996585ef3e903842048 Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 19 Mar 2025 10:27:10 +0100 Subject: [PATCH 06/14] remove duplicate input_truncated on test --- packages/dd-trace/test/appsec/telemetry/rasp.spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/dd-trace/test/appsec/telemetry/rasp.spec.js b/packages/dd-trace/test/appsec/telemetry/rasp.spec.js index a8baca4be1d..fcd20b78607 100644 --- a/packages/dd-trace/test/appsec/telemetry/rasp.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/rasp.spec.js @@ -146,8 +146,7 @@ describe('Appsec Rasp Telemetry metrics', () => { waf_error: false, waf_timeout: false, waf_version: wafVersion, - event_rules_version: rulesVersion, - input_truncated: false + event_rules_version: rulesVersion }) }) }) From 82629a7ccf69bf21f56ed2d7eef926b908677c3e Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 19 Mar 2025 11:03:30 +0100 Subject: [PATCH 07/14] fix waf requests telemtry test --- .../dd-trace/test/appsec/telemetry/waf.spec.js | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index 0684098a321..c57da59ccf3 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -272,7 +272,8 @@ describe('Appsec Waf Telemetry metrics', () => { appsecTelemetry.incrementWafRequestsMetric(req) - expect(count).to.have.been.calledOnceWithExactly('waf.requests', { + expect(count).to.have.been.calledWithExactly('waf.input_truncated', { truncation_reason: 1 }) + expect(count).to.have.been.calledWithExactly('waf.requests', { request_blocked: true, block_failure: true, rule_triggered: true, @@ -283,18 +284,6 @@ describe('Appsec Waf Telemetry metrics', () => { waf_version: wafVersion, event_rules_version: rulesVersion }) - expect(count).to.have.been.calledOnce - expect(count.firstCall.args[1]).to.deep.equal({ - block_failure: true, - input_truncated: true, - request_blocked: true, - rate_limited: true, - rule_triggered: true, - waf_error: true, - waf_timeout: true, - waf_version: '0.0.1', - event_rules_version: '0.0.2' - }) }) it('should not fail if req has no previous tag', () => { From 0b333858c5fa20513e55cfd6a5b2684db3507797 Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 19 Mar 2025 14:16:55 +0100 Subject: [PATCH 08/14] fix rate limiter --- packages/dd-trace/src/appsec/reporter.js | 3 +-- packages/dd-trace/test/appsec/reporter.spec.js | 3 +-- packages/dd-trace/test/appsec/waf-metrics.integration.spec.js | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 4e667cc4559..3efc70709a8 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -148,6 +148,7 @@ function reportAttack (attackData) { if (limiter.isAllowed()) { keepTrace(rootSpan, ASM) + } else { updateWafRateLimitedMetric(req) } @@ -204,8 +205,6 @@ function finishRequest (req, res) { rootSpan.addTags(Object.fromEntries(metricsQueue)) keepTrace(rootSpan, ASM) - updateWafRateLimitedMetric(req) - metricsQueue.clear() } diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index b199365fb33..d6a527003c2 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -757,7 +757,7 @@ describe('reporter', () => { expect(span.setTag).to.have.been.calledWithExactly('_dd.appsec.rasp.error', -1) }) - it('should keep span and call updateWafRateLimitedMetric if there are metrics', () => { + it('should keep span if there are metrics', () => { const req = {} Reporter.metricsQueue.set('a', 1) @@ -766,7 +766,6 @@ describe('reporter', () => { Reporter.finishRequest(req, wafContext, {}) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) }) }) }) diff --git a/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js index 991f0589905..f4f7dc0f1e3 100644 --- a/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js +++ b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js @@ -79,7 +79,7 @@ describe('WAF Metrics', () => { assert.exists(wafRequests, 'Waf requests serie should exist') assert.strictEqual(wafRequests.type, 'count') assert.include(wafRequests.tags, 'waf_error:true') - assert.include(wafRequests.tags, 'rate_limited:true') + assert.include(wafRequests.tags, 'rate_limited:false') } }, 30_000, 'generate-metrics', 2) From a5534458f9f82390d6e64bc2562c922c548b11f6 Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 19 Mar 2025 14:42:44 +0100 Subject: [PATCH 09/14] report waf error metric --- packages/dd-trace/src/appsec/telemetry/waf.js | 1 + .../dd-trace/test/appsec/telemetry/waf.spec.js | 15 +++++++++++++-- .../test/appsec/waf-metrics.integration.spec.js | 5 +++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index efa56f2316e..a04c4d3fd1c 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -72,6 +72,7 @@ function trackWafMetrics (store, metrics) { if (metrics.errorCode) { metricTags[tags.WAF_ERROR] = true + appsecMetrics.count('waf.error', { ...versionsTags, waf_error: metrics.errorCode }).inc() } if (metrics.wafTimeout) { diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index c57da59ccf3..1bdedaeaaea 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -192,8 +192,19 @@ describe('Appsec Waf Telemetry metrics', () => { }) it('should keep the maximum wafErrorCode', () => { - appsecTelemetry.updateWafRequestsMetricTags({ errorCode: -1 }, req) - appsecTelemetry.updateWafRequestsMetricTags({ errorCode: -3 }, req) + appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion, errorCode: -1 }, req) + expect(count).to.have.been.calledWithExactly('waf.error', { + waf_version: wafVersion, + event_rules_version: rulesVersion, + waf_error: -1 + }) + + appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion, errorCode: -3 }, req) + expect(count).to.have.been.calledWithExactly('waf.error', { + waf_version: wafVersion, + event_rules_version: rulesVersion, + waf_error: -3 + }) const { wafErrorCode } = appsecTelemetry.getRequestMetrics(req) expect(wafErrorCode).to.equal(-1) diff --git a/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js index f4f7dc0f1e3..268d3c972b7 100644 --- a/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js +++ b/packages/dd-trace/test/appsec/waf-metrics.integration.spec.js @@ -80,6 +80,11 @@ describe('WAF Metrics', () => { assert.strictEqual(wafRequests.type, 'count') assert.include(wafRequests.tags, 'waf_error:true') assert.include(wafRequests.tags, 'rate_limited:false') + + const wafError = series.find(s => s.metric === 'waf.error') + assert.exists(wafError, 'Waf error serie should exist') + assert.strictEqual(wafError.type, 'count') + assert.include(wafError.tags, 'waf_error:-127') } }, 30_000, 'generate-metrics', 2) From 628133c2995cfae601d0a738485368bfac8d7fb2 Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 19 Mar 2025 14:52:26 +0100 Subject: [PATCH 10/14] fix updateWafRateLimitedMetric reporter test --- .../dd-trace/test/appsec/reporter.spec.js | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index d6a527003c2..69b665e3153 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -288,7 +288,7 @@ describe('reporter', () => { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called }) it('should add tags to request span', () => { @@ -303,7 +303,7 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called }) it('should not add manual.keep when rate limit is reached', (done) => { @@ -314,24 +314,24 @@ describe('reporter', () => { expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(3) - expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(3) + expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called Reporter.setRateLimit(1) expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) - expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(4) + expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) - expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(4) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) setTimeout(() => { expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(5) - expect(telemetry.updateWafRateLimitedMetric).to.be.callCount(5) + expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) done() }, 1020) }) @@ -349,7 +349,7 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called }) it('should merge attacks json', () => { @@ -366,10 +366,10 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called }) - it('should call standalone sample and updateWafRateLimitedMetric', () => { + it('should call standalone sample', () => { span.context()._tags = { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' } const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]') @@ -384,7 +384,7 @@ describe('reporter', () => { }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called }) }) From 21f6acea62e05ebe1a41e890beb2d2571c7a6d8c Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 20 Mar 2025 11:47:07 +0100 Subject: [PATCH 11/14] fix functions order --- packages/dd-trace/src/appsec/reporter.js | 1 + packages/dd-trace/src/appsec/telemetry/index.js | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 5dc8b6562e5..83f7031699f 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -207,6 +207,7 @@ function finishRequest (req, res) { rootSpan.addTags(Object.fromEntries(metricsQueue)) keepTrace(rootSpan, ASM) + metricsQueue.clear() } diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index bc04eb4f47f..d8a14fce58a 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -133,14 +133,14 @@ module.exports = { disable, updateWafRequestsMetricTags, + updateWafRateLimitedMetric, + updateWafBlockFailureMetric, updateRaspRequestsMetricTags, incrementWafInitMetric, incrementWafUpdatesMetric, incrementWafRequestsMetric, incrementMissingUserLoginMetric, incrementMissingUserIdMetric, - updateWafRateLimitedMetric, - updateWafBlockFailureMetric, getRequestMetrics } From 4fc09e381f7505b1269937f6987bc899ea2b98d6 Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 24 Mar 2025 10:51:07 +0100 Subject: [PATCH 12/14] change function names --- packages/dd-trace/src/appsec/blocking.js | 4 ++-- packages/dd-trace/src/appsec/graphql.js | 4 ++-- packages/dd-trace/src/appsec/reporter.js | 4 ++-- .../dd-trace/src/appsec/telemetry/index.js | 8 ++++---- .../dd-trace/test/appsec/blocking.spec.js | 14 ++++++------- packages/dd-trace/test/appsec/graphql.spec.js | 6 +++--- .../dd-trace/test/appsec/reporter.spec.js | 20 +++++++++---------- .../test/appsec/telemetry/waf.spec.js | 12 +++++------ 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/dd-trace/src/appsec/blocking.js b/packages/dd-trace/src/appsec/blocking.js index 5e0090838d6..ab9bafc3ae2 100644 --- a/packages/dd-trace/src/appsec/blocking.js +++ b/packages/dd-trace/src/appsec/blocking.js @@ -2,7 +2,7 @@ const log = require('../log') const blockedTemplates = require('./blocked_templates') -const { updateWafBlockFailureMetric } = require('./telemetry') +const { updateBlockFailureMetric } = require('./telemetry') const detectedSpecificEndpoints = {} @@ -129,7 +129,7 @@ function block (req, res, rootSpan, abortController, actionParameters = defaultB rootSpan?.setTag('_dd.appsec.block.failed', 1) log.error('[ASM] Blocking error', err) - updateWafBlockFailureMetric(req) + updateBlockFailureMetric(req) return false } } diff --git a/packages/dd-trace/src/appsec/graphql.js b/packages/dd-trace/src/appsec/graphql.js index bda5bb2bea9..4799bab6ce9 100644 --- a/packages/dd-trace/src/appsec/graphql.js +++ b/packages/dd-trace/src/appsec/graphql.js @@ -17,7 +17,7 @@ const { apolloChannel, apolloServerCoreChannel } = require('./channels') -const { updateWafBlockFailureMetric } = require('./telemetry') +const { updateBlockFailureMetric } = require('./telemetry') const graphqlRequestData = new WeakMap() @@ -109,7 +109,7 @@ function beforeWriteApolloGraphqlResponse ({ abortController, abortData }) { rootSpan.setTag('_dd.appsec.block.failed', 1) log.error('[ASM] Blocking error', err) - updateWafBlockFailureMetric(req) + updateBlockFailureMetric(req) } } diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 83f7031699f..ba7ceaa7b0c 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -11,7 +11,7 @@ const { incrementWafUpdatesMetric, incrementWafRequestsMetric, getRequestMetrics, - updateWafRateLimitedMetric + updateRateLimitedMetric } = require('./telemetry') const zlib = require('zlib') const { keepTrace } = require('../priority_sampler') @@ -151,7 +151,7 @@ function reportAttack (attackData) { if (limiter.isAllowed()) { keepTrace(rootSpan, ASM) } else { - updateWafRateLimitedMetric(req) + updateRateLimitedMetric(req) } // TODO: maybe add this to format.js later (to take decision as late as possible) diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index d8a14fce58a..18a3e7ca0ec 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -74,14 +74,14 @@ function updateWafRequestsMetricTags (metrics, req) { return trackWafMetrics(store, metrics) } -function updateWafRateLimitedMetric (req) { +function updateRateLimitedMetric (req) { if (!enabled) return const store = getStore(req) trackWafMetrics(store, { rateLimited: true }) } -function updateWafBlockFailureMetric (req) { +function updateBlockFailureMetric (req) { if (!enabled) return const store = getStore(req) @@ -133,8 +133,8 @@ module.exports = { disable, updateWafRequestsMetricTags, - updateWafRateLimitedMetric, - updateWafBlockFailureMetric, + updateRateLimitedMetric, + updateBlockFailureMetric, updateRaspRequestsMetricTags, incrementWafInitMetric, incrementWafUpdatesMetric, diff --git a/packages/dd-trace/test/appsec/blocking.spec.js b/packages/dd-trace/test/appsec/blocking.spec.js index 03905de5a14..a90d30dc9af 100644 --- a/packages/dd-trace/test/appsec/blocking.spec.js +++ b/packages/dd-trace/test/appsec/blocking.spec.js @@ -23,7 +23,7 @@ describe('blocking', () => { } telemetry = { - updateWafBlockFailureMetric: sinon.stub() + updateBlockFailureMetric: sinon.stub() } const blocking = proxyquire('../src/appsec/blocking', { @@ -71,7 +71,7 @@ describe('blocking', () => { expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1) expect(res.setHeader).to.not.have.been.called expect(res.constructor.prototype.end).to.not.have.been.called - expect(telemetry.updateWafBlockFailureMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateBlockFailureMetric).to.be.calledOnceWithExactly(req) }) it('should send blocking response with html type if present in the headers', () => { @@ -85,7 +85,7 @@ describe('blocking', () => { 'Content-Length': 12 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('htmlBodyéé') - expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called }) it('should send blocking response with json type if present in the headers in priority', () => { @@ -99,7 +99,7 @@ describe('blocking', () => { 'Content-Length': 8 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') - expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called }) it('should send blocking response with json type if neither html or json is present in the headers', () => { @@ -112,7 +112,7 @@ describe('blocking', () => { 'Content-Length': 8 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') - expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called }) it('should send blocking response and call abortController if passed in arguments', () => { @@ -127,7 +127,7 @@ describe('blocking', () => { }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') expect(abortController.signal.aborted).to.be.true - expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called }) it('should remove all headers before sending blocking response', () => { @@ -145,7 +145,7 @@ describe('blocking', () => { 'Content-Length': 8 }) expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') - expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called }) }) diff --git a/packages/dd-trace/test/appsec/graphql.spec.js b/packages/dd-trace/test/appsec/graphql.spec.js index a31ca2af1d4..9b6d6380e9e 100644 --- a/packages/dd-trace/test/appsec/graphql.spec.js +++ b/packages/dd-trace/test/appsec/graphql.spec.js @@ -29,7 +29,7 @@ describe('GraphQL', () => { }) telemetry = { - updateWafBlockFailureMetric: sinon.stub() + updateBlockFailureMetric: sinon.stub() } graphql = proxyquire('../../src/appsec/graphql', { @@ -238,7 +238,7 @@ describe('GraphQL', () => { expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters) expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('appsec.blocked', 'true') - expect(telemetry.updateWafBlockFailureMetric).to.not.have.been.called + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called }) it('Should catch error when block fails', () => { @@ -268,7 +268,7 @@ describe('GraphQL', () => { expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters) expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1) - expect(telemetry.updateWafBlockFailureMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateBlockFailureMetric).to.be.calledOnceWithExactly(req) }) }) }) diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index 55989779411..c5575c0baae 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -38,7 +38,7 @@ describe('reporter', () => { updateRaspRequestsMetricTags: sinon.stub(), incrementWafUpdatesMetric: sinon.stub(), incrementWafRequestsMetric: sinon.stub(), - updateWafRateLimitedMetric: sinon.stub(), + updateRateLimitedMetric: sinon.stub(), getRequestMetrics: sinon.stub() } @@ -297,7 +297,7 @@ describe('reporter', () => { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called + expect(telemetry.updateRateLimitedMetric).to.not.have.been.called }) it('should add tags to request span', () => { @@ -312,7 +312,7 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called + expect(telemetry.updateRateLimitedMetric).to.not.have.been.called }) it('should not add manual.keep when rate limit is reached', (done) => { @@ -323,24 +323,24 @@ describe('reporter', () => { expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(3) - expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called + expect(telemetry.updateRateLimitedMetric).to.not.have.been.called Reporter.setRateLimit(1) expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) - expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called + expect(telemetry.updateRateLimitedMetric).to.not.have.been.called expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req) setTimeout(() => { expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(5) - expect(telemetry.updateWafRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req) done() }, 1020) }) @@ -358,7 +358,7 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called + expect(telemetry.updateRateLimitedMetric).to.not.have.been.called }) it('should merge attacks json', () => { @@ -375,7 +375,7 @@ describe('reporter', () => { 'network.client.ip': '8.8.8.8' }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called + expect(telemetry.updateRateLimitedMetric).to.not.have.been.called }) it('should call standalone sample', () => { @@ -393,7 +393,7 @@ describe('reporter', () => { }) expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM) - expect(telemetry.updateWafRateLimitedMetric).to.not.have.been.called + expect(telemetry.updateRateLimitedMetric).to.not.have.been.called }) }) diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index 71327dbb63a..42718951a99 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -342,17 +342,17 @@ describe('Appsec Waf Telemetry metrics', () => { }) }) - describe('updateWafRateLimitedMetric', () => { + describe('updateRateLimitedMetric', () => { it('should set rate_limited to true on the request tags', () => { - appsecTelemetry.updateWafRateLimitedMetric(req) + appsecTelemetry.updateRateLimitedMetric(req) const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) expect(result.rate_limited).to.be.true }) }) - describe('updateWafBlockFailureMetric', () => { + describe('updateBlockFailureMetric', () => { it('should set block_failure to true on the request tags', () => { - appsecTelemetry.updateWafBlockFailureMetric(req) + appsecTelemetry.updateBlockFailureMetric(req) const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) expect(result.block_failure).to.be.true }) @@ -442,13 +442,13 @@ describe('Appsec Waf Telemetry metrics', () => { }) it('should not set rate_limited if telemetry is disabled', () => { - appsecTelemetry.updateWafRateLimitedMetric(req) + appsecTelemetry.updateRateLimitedMetric(req) const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) expect(result).to.be.undefined }) it('should not set block_failure if telemetry is disabled', () => { - appsecTelemetry.updateWafBlockFailureMetric(req) + appsecTelemetry.updateBlockFailureMetric(req) const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) expect(result).to.be.undefined }) From e0f36676dc4eab9b92e14f9bb0a3b20c9d91d27d Mon Sep 17 00:00:00 2001 From: ishabi Date: Tue, 25 Mar 2025 13:52:04 +0100 Subject: [PATCH 13/14] add versions to report attack --- packages/dd-trace/src/appsec/reporter.js | 4 +-- .../dd-trace/src/appsec/telemetry/index.js | 4 +-- packages/dd-trace/src/appsec/telemetry/waf.js | 16 +++------ .../src/appsec/waf/waf_context_wrapper.js | 5 ++- .../dd-trace/test/appsec/reporter.spec.js | 35 +++++++++++-------- .../test/appsec/telemetry/waf.spec.js | 4 +-- .../dd-trace/test/appsec/waf/index.spec.js | 5 ++- 7 files changed, 39 insertions(+), 34 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index ba7ceaa7b0c..0cc30d6e078 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -136,7 +136,7 @@ function reportTruncationMetrics (rootSpan, metrics) { } } -function reportAttack (attackData) { +function reportAttack (attackData, { rulesVersion, wafVersion }) { const store = storage('legacy').getStore() const req = store?.req const rootSpan = web.root(req) @@ -151,7 +151,7 @@ function reportAttack (attackData) { if (limiter.isAllowed()) { keepTrace(rootSpan, ASM) } else { - updateRateLimitedMetric(req) + updateRateLimitedMetric(req, { rulesVersion, wafVersion }) } // TODO: maybe add this to format.js later (to take decision as late as possible) diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index 18a3e7ca0ec..038a9cdf527 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -74,11 +74,11 @@ function updateWafRequestsMetricTags (metrics, req) { return trackWafMetrics(store, metrics) } -function updateRateLimitedMetric (req) { +function updateRateLimitedMetric (req, { rulesVersion, wafVersion }) { if (!enabled) return const store = getStore(req) - trackWafMetrics(store, { rateLimited: true }) + trackWafMetrics(store, { rateLimited: true, rulesVersion, wafVersion }) } function updateBlockFailureMetric (req) { diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index 83dd6a82ac5..5571c9cfa90 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -48,16 +48,12 @@ function trackWafMetrics (store, metrics) { trackWafDurations(metrics, versionsTags) - const metricTags = getOrCreateMetricTags(store) + const metricTags = getOrCreateMetricTags(store, versionsTags) if (metrics.blockFailed) { metricTags[tags.BLOCK_FAILURE] = true } - if (versionsTags[tags.EVENT_RULES_VERSION]) { - metricTags[tags.EVENT_RULES_VERSION] = versionsTags[tags.EVENT_RULES_VERSION] - } - if (metrics.blockTriggered) { metricTags[tags.REQUEST_BLOCKED] = true } @@ -79,10 +75,6 @@ function trackWafMetrics (store, metrics) { metricTags[tags.WAF_TIMEOUT] = true } - if (versionsTags[tags.WAF_VERSION]) { - metricTags[tags.WAF_VERSION] = versionsTags[tags.WAF_VERSION] - } - const truncationReason = getTruncationReason(metrics) if (truncationReason > 0) { metricTags[tags.INPUT_TRUNCATED] = true @@ -92,20 +84,20 @@ function trackWafMetrics (store, metrics) { return metricTags } -function getOrCreateMetricTags (store) { +function getOrCreateMetricTags (store, versionsTags) { let metricTags = store[DD_TELEMETRY_WAF_RESULT_TAGS] if (!metricTags) { metricTags = { [tags.BLOCK_FAILURE]: false, - [tags.EVENT_RULES_VERSION]: null, [tags.INPUT_TRUNCATED]: false, [tags.RATE_LIMITED]: false, [tags.REQUEST_BLOCKED]: false, [tags.RULE_TRIGGERED]: false, [tags.WAF_ERROR]: false, [tags.WAF_TIMEOUT]: false, - [tags.WAF_VERSION]: null + + ...versionsTags } store[DD_TELEMETRY_WAF_RESULT_TAGS] = metricTags } diff --git a/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js b/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js index 07f127bf00d..334441e4027 100644 --- a/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js +++ b/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js @@ -133,7 +133,10 @@ class WAFContextWrapper { metrics.wafTimeout = result.timeout if (ruleTriggered) { - Reporter.reportAttack(JSON.stringify(result.events)) + Reporter.reportAttack(JSON.stringify(result.events), { + rulesVersion: this.rulesVersion, + wafVersion: this.wafVersion + }) } Reporter.reportDerivatives(result.derivatives) diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index c5575c0baae..3879dc8915c 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -264,7 +264,14 @@ describe('reporter', () => { }) describe('reportAttack', () => { - let req + let req, metrics + + before(() => { + metrics = { + rulesVersion: '1.2.3', + wafVersion: '4.5.6' + } + }) beforeEach(() => { req = { @@ -286,7 +293,7 @@ describe('reporter', () => { it('should add tags to request span when socket is not there', () => { delete req.socket - const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]') + const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]', metrics) expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -301,7 +308,7 @@ describe('reporter', () => { }) it('should add tags to request span', () => { - const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]') + const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]', metrics) expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -318,29 +325,29 @@ describe('reporter', () => { it('should not add manual.keep when rate limit is reached', (done) => { const addTags = span.addTags - expect(Reporter.reportAttack('')).to.not.be.false - expect(Reporter.reportAttack('')).to.not.be.false - expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('', metrics)).to.not.be.false + expect(Reporter.reportAttack('', metrics)).to.not.be.false + expect(Reporter.reportAttack('', metrics)).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(3) expect(telemetry.updateRateLimitedMetric).to.not.have.been.called Reporter.setRateLimit(1) - expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('', metrics)).to.not.be.false expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) expect(telemetry.updateRateLimitedMetric).to.not.have.been.called - expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('', metrics)).to.not.be.false expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) - expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req, metrics) setTimeout(() => { - expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('', metrics)).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(5) - expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req) + expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req, metrics) done() }, 1020) }) @@ -348,7 +355,7 @@ describe('reporter', () => { it('should not overwrite origin tag', () => { span.context()._tags = { '_dd.origin': 'tracer' } - const result = Reporter.reportAttack('[]') + const result = Reporter.reportAttack('[]', metrics) expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -364,7 +371,7 @@ describe('reporter', () => { it('should merge attacks json', () => { span.context()._tags = { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' } - const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]') + const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]', metrics) expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -381,7 +388,7 @@ describe('reporter', () => { it('should call standalone sample', () => { span.context()._tags = { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' } - const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]') + const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]', metrics) expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index 42718951a99..d25f80ab112 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -344,7 +344,7 @@ describe('Appsec Waf Telemetry metrics', () => { describe('updateRateLimitedMetric', () => { it('should set rate_limited to true on the request tags', () => { - appsecTelemetry.updateRateLimitedMetric(req) + appsecTelemetry.updateRateLimitedMetric(req, metrics) const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) expect(result.rate_limited).to.be.true }) @@ -442,7 +442,7 @@ describe('Appsec Waf Telemetry metrics', () => { }) it('should not set rate_limited if telemetry is disabled', () => { - appsecTelemetry.updateRateLimitedMetric(req) + appsecTelemetry.updateRateLimitedMetric(req, { wafVersion, rulesVersion }) const result = appsecTelemetry.updateWafRequestsMetricTags({ wafVersion, rulesVersion }, req) expect(result).to.be.undefined }) diff --git a/packages/dd-trace/test/appsec/waf/index.spec.js b/packages/dd-trace/test/appsec/waf/index.spec.js index 6cd03abc005..fdc5187c4b5 100644 --- a/packages/dd-trace/test/appsec/waf/index.spec.js +++ b/packages/dd-trace/test/appsec/waf/index.spec.js @@ -325,7 +325,10 @@ describe('WAF Manager', () => { wafContextWrapper.run(params) - expect(Reporter.reportAttack).to.be.calledOnceWithExactly('["ATTACK DATA"]') + expect(Reporter.reportAttack).to.be.calledOnceWithExactly( + '["ATTACK DATA"]', + { rulesVersion: '1.0.0', wafVersion: '1.2.3' } + ) }) it('should report if rule is triggered', () => { From f81fc9b2b659c3cfb9dcb1403065dff42bfccf8d Mon Sep 17 00:00:00 2001 From: ishabi Date: Tue, 25 Mar 2025 14:18:09 +0100 Subject: [PATCH 14/14] fix reporting rate limiting metric --- packages/dd-trace/src/appsec/reporter.js | 4 +-- .../dd-trace/src/appsec/telemetry/index.js | 4 +-- .../src/appsec/waf/waf_context_wrapper.js | 5 +-- .../dd-trace/test/appsec/reporter.spec.js | 35 ++++++++----------- .../dd-trace/test/appsec/waf/index.spec.js | 5 +-- 5 files changed, 20 insertions(+), 33 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 0cc30d6e078..ba7ceaa7b0c 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -136,7 +136,7 @@ function reportTruncationMetrics (rootSpan, metrics) { } } -function reportAttack (attackData, { rulesVersion, wafVersion }) { +function reportAttack (attackData) { const store = storage('legacy').getStore() const req = store?.req const rootSpan = web.root(req) @@ -151,7 +151,7 @@ function reportAttack (attackData, { rulesVersion, wafVersion }) { if (limiter.isAllowed()) { keepTrace(rootSpan, ASM) } else { - updateRateLimitedMetric(req, { rulesVersion, wafVersion }) + updateRateLimitedMetric(req) } // TODO: maybe add this to format.js later (to take decision as late as possible) diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index 038a9cdf527..18a3e7ca0ec 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -74,11 +74,11 @@ function updateWafRequestsMetricTags (metrics, req) { return trackWafMetrics(store, metrics) } -function updateRateLimitedMetric (req, { rulesVersion, wafVersion }) { +function updateRateLimitedMetric (req) { if (!enabled) return const store = getStore(req) - trackWafMetrics(store, { rateLimited: true, rulesVersion, wafVersion }) + trackWafMetrics(store, { rateLimited: true }) } function updateBlockFailureMetric (req) { diff --git a/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js b/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js index 334441e4027..07f127bf00d 100644 --- a/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js +++ b/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js @@ -133,10 +133,7 @@ class WAFContextWrapper { metrics.wafTimeout = result.timeout if (ruleTriggered) { - Reporter.reportAttack(JSON.stringify(result.events), { - rulesVersion: this.rulesVersion, - wafVersion: this.wafVersion - }) + Reporter.reportAttack(JSON.stringify(result.events)) } Reporter.reportDerivatives(result.derivatives) diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index 3879dc8915c..c5575c0baae 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -264,14 +264,7 @@ describe('reporter', () => { }) describe('reportAttack', () => { - let req, metrics - - before(() => { - metrics = { - rulesVersion: '1.2.3', - wafVersion: '4.5.6' - } - }) + let req beforeEach(() => { req = { @@ -293,7 +286,7 @@ describe('reporter', () => { it('should add tags to request span when socket is not there', () => { delete req.socket - const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]', metrics) + const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]') expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -308,7 +301,7 @@ describe('reporter', () => { }) it('should add tags to request span', () => { - const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]', metrics) + const result = Reporter.reportAttack('[{"rule":{},"rule_matches":[{}]}]') expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -325,29 +318,29 @@ describe('reporter', () => { it('should not add manual.keep when rate limit is reached', (done) => { const addTags = span.addTags - expect(Reporter.reportAttack('', metrics)).to.not.be.false - expect(Reporter.reportAttack('', metrics)).to.not.be.false - expect(Reporter.reportAttack('', metrics)).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(3) expect(telemetry.updateRateLimitedMetric).to.not.have.been.called Reporter.setRateLimit(1) - expect(Reporter.reportAttack('', metrics)).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) expect(telemetry.updateRateLimitedMetric).to.not.have.been.called - expect(Reporter.reportAttack('', metrics)).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true') expect(prioritySampler.setPriority).to.have.callCount(4) - expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req, metrics) + expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req) setTimeout(() => { - expect(Reporter.reportAttack('', metrics)).to.not.be.false + expect(Reporter.reportAttack('')).to.not.be.false expect(prioritySampler.setPriority).to.have.callCount(5) - expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req, metrics) + expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req) done() }, 1020) }) @@ -355,7 +348,7 @@ describe('reporter', () => { it('should not overwrite origin tag', () => { span.context()._tags = { '_dd.origin': 'tracer' } - const result = Reporter.reportAttack('[]', metrics) + const result = Reporter.reportAttack('[]') expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -371,7 +364,7 @@ describe('reporter', () => { it('should merge attacks json', () => { span.context()._tags = { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' } - const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]', metrics) + const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]') expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) @@ -388,7 +381,7 @@ describe('reporter', () => { it('should call standalone sample', () => { span.context()._tags = { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' } - const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]', metrics) + const result = Reporter.reportAttack('[{"rule":{}},{"rule":{},"rule_matches":[{}]}]') expect(result).to.not.be.false expect(web.root).to.have.been.calledOnceWith(req) diff --git a/packages/dd-trace/test/appsec/waf/index.spec.js b/packages/dd-trace/test/appsec/waf/index.spec.js index fdc5187c4b5..6cd03abc005 100644 --- a/packages/dd-trace/test/appsec/waf/index.spec.js +++ b/packages/dd-trace/test/appsec/waf/index.spec.js @@ -325,10 +325,7 @@ describe('WAF Manager', () => { wafContextWrapper.run(params) - expect(Reporter.reportAttack).to.be.calledOnceWithExactly( - '["ATTACK DATA"]', - { rulesVersion: '1.0.0', wafVersion: '1.2.3' } - ) + expect(Reporter.reportAttack).to.be.calledOnceWithExactly('["ATTACK DATA"]') }) it('should report if rule is triggered', () => {