fix stack trace tests

DataDog · Dec 23, 2024 · 3cc25af · 3cc25af
1 parent f6edc6e
commit 3cc25af
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 36 deletions.
diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js
@@ -3,7 +3,7 @@
 const { storage } = require('../../../../../datadog-core')
 const { getFirstNonDDPathAndLine } = require('../path-line')
 const { addVulnerability, getVulnerabilityCallSiteList } = require('../vulnerability-reporter')
-const { getIastContext } = require('../iast-context')
+const { getIastContext, getIastStackTraceId } = require('../iast-context')
 const overheadController = require('../overhead-controller')
 const { SinkIastPlugin } = require('../iast-plugin')
 const { getOriginalPathAndLineFromSourceMap } = require('../taint-tracking/rewriter')
@@ -34,8 +34,9 @@ class Analyzer extends SinkIastPlugin {
     if (!this._isExcluded(location)) {
       const locationSourceMap = this._replaceLocationFromSourceMap(location)
       const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId()
-      const vulnerability = this._createVulnerability(this._type, evidence, spanId, locationSourceMap)
-      addVulnerability(context, vulnerability, callSiteList)
+      const stackId = getIastStackTraceId(context)
+      const vulnerability = this._createVulnerability(this._type, evidence, spanId, locationSourceMap, stackId)
+      addVulnerability(context, vulnerability, callSiteList, stackId)
     }
   }
 
@@ -104,12 +105,13 @@ class Analyzer extends SinkIastPlugin {
     return overheadController.hasQuota(overheadController.OPERATIONS.REPORT_VULNERABILITY, context)
   }
 
-  _createVulnerability (type, evidence, spanId, location) {
+  _createVulnerability (type, evidence, spanId, location, stackId) {
     if (type && evidence) {
       const _spanId = spanId || 0
       return {
         type,
         evidence,
+        stackId,
         location: {
           spanId: _spanId,
           ...location

diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js
@@ -7,7 +7,6 @@ const standalone = require('../standalone')
 const { SAMPLING_MECHANISM_APPSEC } = require('../../constants')
 const { keepTrace } = require('../../priority_sampler')
 const { reportStackTrace, getCallSiteList, STACK_TRACE_NAMESPACES } = require('../stack_trace')
-const { getIastStackTraceId } = require('./iast-context')
 
 const VULNERABILITIES_KEY = 'vulnerabilities'
 const VULNERABILITY_HASHES_MAX_SIZE = 1000
@@ -20,7 +19,7 @@ let deduplicationEnabled = true
 let stackTraceMaxDepth
 let maxStackTraces
 
-function addVulnerability (iastContext, vulnerability, callSiteList) {
+function addVulnerability (iastContext, vulnerability, callSiteList, stackId) {
   if (vulnerability?.evidence && vulnerability?.type && vulnerability?.location) {
     if (deduplicationEnabled && isDuplicatedVulnerability(vulnerability)) return
 
@@ -45,9 +44,6 @@ function addVulnerability (iastContext, vulnerability, callSiteList) {
     keepTrace(span, SAMPLING_MECHANISM_APPSEC)
     standalone.sample(span)
 
-    const stackId = getIastStackTraceId(iastContext)
-    vulnerability.stackId = stackId
-
     reportStackTrace(
       iastContext?.rootSpan,
       stackId,

diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js
@@ -31,11 +31,11 @@ class DatadogRaspAbortError extends Error {
 function handleResult (actions, req, res, abortController, config) {
   const generateStackTraceAction = actions?.generate_stack
 
-  const { maxDepth, maxStackTraces } = config.appsec.stackTrace
+  const { enabled, maxDepth, maxStackTraces } = config.appsec.stackTrace
 
   const callSiteList = getCallSiteList(maxDepth)
 
-  if (generateStackTraceAction && config.appsec.stackTrace.enabled) {
+  if (generateStackTraceAction && enabled) {
     const rootSpan = web.root(req)
     reportStackTrace(
       rootSpan,

diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js
@@ -12,9 +12,7 @@ const STACK_TRACE_NAMESPACES = {
 }
 
 function getCallSiteList (maxDepth = 100) {
-  if (maxDepth < 1) {
-    maxDepth = Infinity
-  }
+  if (maxDepth < 1) maxDepth = Infinity
 
   const previousPrepareStackTrace = Error.prepareStackTrace
   const previousStackTraceLimit = Error.stackTraceLimit
@@ -68,6 +66,7 @@ function reportStackTrace (
   if (!rootSpan) return
 
   if (maxStackTraces < 1 || (rootSpan.meta_struct?.['_dd.stack']?.[namespace]?.length ?? 0) < maxStackTraces) {
+    if (maxDepth < 1) maxDepth = Infinity
     if (!Array.isArray(callSiteList)) return
 
     if (!rootSpan.meta_struct) {

diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js
@@ -21,7 +21,8 @@ describe('vulnerability-analyzer', () => {
   beforeEach(() => {
     vulnerabilityReporter = {
       createVulnerability: sinon.stub().returns(VULNERABILITY),
-      addVulnerability: sinon.stub()
+      addVulnerability: sinon.stub(),
+      getVulnerabilityCallSiteList: sinon.stub().returns([])
     }
     pathLine = {
       getFirstNonDDPathAndLine: sinon.stub().returns(VULNERABILITY_LOCATION)
@@ -116,10 +117,11 @@ describe('vulnerability-analyzer', () => {
       }
     }
     vulnerabilityAnalyzer._report(VULNERABLE_VALUE, context)
-    expect(vulnerabilityReporter.addVulnerability).to.have.been.calledOnceWithExactly(
+    expect(vulnerabilityReporter.addVulnerability).to.have.been.calledOnceWith(
       context,
       {
         type: 'TEST_ANALYZER',
+        stackId: 1,
         evidence: {
           value: 'VULNERABLE_VALUE'
         },
@@ -129,7 +131,9 @@ describe('vulnerability-analyzer', () => {
           line: 42
         },
         hash: 5975567724
-      }
+      },
+      [],
+      1
     )
   })
 

diff --git a/packages/dd-trace/test/appsec/iast/path-line.spec.js b/packages/dd-trace/test/appsec/iast/path-line.spec.js
@@ -52,7 +52,11 @@ describe('path-line', function () {
 
   describe('getFirstNonDDPathAndLine', () => {
     it('call does not fail', () => {
-      const obj = pathLine.getFirstNonDDPathAndLine()
+      const PROJECT_PATH = path.join(rootPath, 'project-path')
+      const DD_BASE_PATH = path.join(PROJECT_PATH, 'node_modules', 'dd-trace')
+      const callSiteList = []
+      callSiteList.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89))
+      const obj = pathLine.getFirstNonDDPathAndLine(callSiteList, false)
       expect(obj).to.not.be.null
     })
   })

diff --git a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js
@@ -107,6 +107,13 @@ describe('vulnerability-reporter', () => {
         start({
           iast: {
             deduplicationEnabled: true
+          },
+          appsec: {
+            stackTrace: {
+              enabled: true,
+              maxStackTraces: 2,
+              maxDepth: 42
+            }
           }
         }, fakeTracer)
       })
@@ -119,15 +126,15 @@ describe('vulnerability-reporter', () => {
       it('should create span on the fly', () => {
         const vulnerability =
           vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, undefined,
-            { path: 'filename.js', line: 73 })
+            { path: 'filename.js', line: 73 }, 1)
         addVulnerability(undefined, vulnerability)
         expect(fakeTracer.startSpan).to.have.been.calledOnceWithExactly('vulnerability', { type: 'vulnerability' })
         expect(onTheFlySpan.addTags.firstCall).to.have.been.calledWithExactly({
           '_dd.iast.enabled': 1
         })
         expect(onTheFlySpan.addTags.secondCall).to.have.been.calledWithExactly({
           '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512655,' +
-            '"evidence":{"value":"sha1"},"location":{"spanId":42,"path":"filename.js","line":73}}]}'
+            '"stackId":1,"evidence":{"value":"sha1"},"location":{"spanId":42,"path":"filename.js","line":73}}]}'
         })
         expect(prioritySampler.setPriority)
           .to.have.been.calledOnceWithExactly(onTheFlySpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC)
@@ -162,6 +169,13 @@ describe('vulnerability-reporter', () => {
       start({
         iast: {
           deduplicationEnabled: true
+        },
+        appsec: {
+          stackTrace: {
+            enabled: true,
+            maxStackTraces: 2,
+            maxDepth: 42
+          }
         }
       })
     })
@@ -386,6 +400,13 @@ describe('vulnerability-reporter', () => {
       start({
         iast: {
           deduplicationEnabled: false
+        },
+        appsec: {
+          stackTrace: {
+            enabled: true,
+            maxStackTraces: 2,
+            maxDepth: 42
+          }
         }
       })
       const iastContext = { rootSpan: span }
@@ -477,7 +498,7 @@ describe('vulnerability-reporter', () => {
       const MAX = 1000
       const vulnerabilityToRepeatInTheNext =
         vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888,
-          { path: 'filename.js', line: 0 })
+          { path: 'filename.js', line: 0 }, 1)
       addVulnerability(iastContext, vulnerabilityToRepeatInTheNext)
       for (let i = 1; i <= MAX; i++) {
         addVulnerability(iastContext,
@@ -497,6 +518,13 @@ describe('vulnerability-reporter', () => {
       const config = {
         iast: {
           deduplicationEnabled: true
+        },
+        appsec: {
+          stackTrace: {
+            enabled: true,
+            maxStackTraces: 2,
+            maxDepth: 42
+          }
         }
       }
       start(config)
@@ -507,6 +535,13 @@ describe('vulnerability-reporter', () => {
       const config = {
         iast: {
           deduplicationEnabled: false
+        },
+        appsec: {
+          stackTrace: {
+            enabled: true,
+            maxStackTraces: 2,
+            maxDepth: 42
+          }
         }
       }
       start(config)
@@ -517,6 +552,13 @@ describe('vulnerability-reporter', () => {
       const config = {
         iast: {
           deduplicationEnabled: true
+        },
+        appsec: {
+          stackTrace: {
+            enabled: true,
+            maxStackTraces: 2,
+            maxDepth: 42
+          }
         }
       }
       start(config)
@@ -542,6 +584,13 @@ describe('vulnerability-reporter', () => {
           redactionEnabled: true,
           redactionNamePattern: null,
           redactionValuePattern: null
+        },
+        appsec: {
+          stackTrace: {
+            enabled: true,
+            maxStackTraces: 2,
+            maxDepth: 42
+          }
         }
       }
       start(config)

diff --git a/packages/dd-trace/test/appsec/rasp/utils.spec.js b/packages/dd-trace/test/appsec/rasp/utils.spec.js
@@ -11,7 +11,8 @@ describe('RASP - utils.js', () => {
     }
 
     stackTrace = {
-      reportStackTrace: sinon.stub()
+      reportStackTrace: sinon.stub(),
+      getCallSiteList: sinon.stub().returns({})
     }
 
     utils = proxyquire('../../../src/appsec/rasp/utils', {
@@ -44,7 +45,7 @@ describe('RASP - utils.js', () => {
       web.root.returns(rootSpan)
 
       utils.handleResult(result, req, undefined, undefined, config)
-      sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 42, 2)
+      sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 42, 2, {})
     })
 
     it('should not report stack trace when no action is present in waf result', () => {