fix: fix AppSec http memory retention (#8029)

* fix: fix AppSec http memory retention

When AppSec is active, http calls have a long memory retention due
to a strong reference behind held. That reference is freed at the
end of a span for example, but it takes additional time and shows
in many memory profiles.

Fix that by using a weak reference and a specific handling to
circumvent that retention issue.

* fix: guarantee runtime metrics client is defined

This is a sync call, so it has never be an issue. If however
anything would report a metric sync from the native module at
construction time, client would have been undefined. Fix that by
moving the client construction before any possible usage.

Author: BridgeAR

packages/datadog-instrumentations/test/passport-http.spec.js

@@ -8,7 +8,7 @@ const { after, before, beforeEach, describe, it } = require('mocha')
 const sinon = require('sinon')
 
 const agent = require('../../dd-trace/test/plugins/agent')
-const { storage } = require('../../datadog-core')
+const { getActiveRequest } = require('../../dd-trace/src/appsec/store')
 const { withVersions } = require('../../dd-trace/test/setup/mocha')
 
 withVersions('passport-http', 'passport-http', version => {
@@ -185,7 +185,8 @@ withVersions('passport-http', 'passport-http', version => {
 
     it('should block when subscriber aborts', async () => {
       subscriberStub = sinon.spy(({ abortController }) => {
-        storage('legacy').getStore().req.res.writeHead(403).end('Blocked')
+        const req = getActiveRequest()
+        req.res.writeHead(403).end('Blocked')
         abortController.abort()
       })
 

packages/datadog-instrumentations/test/passport-local.spec.js

@@ -8,7 +8,7 @@ const sinon = require('sinon')
 
 const axios = require('axios').create({ validateStatus: null })
 const agent = require('../../dd-trace/test/plugins/agent')
-const { storage } = require('../../datadog-core')
+const { getActiveRequest } = require('../../dd-trace/src/appsec/store')
 const { withVersions } = require('../../dd-trace/test/setup/mocha')
 
 withVersions('passport-local', 'passport-local', version => {
@@ -164,7 +164,8 @@ withVersions('passport-local', 'passport-local', version => {
 
     it('should block when subscriber aborts', async () => {
       subscriberStub = sinon.spy(({ abortController }) => {
-        storage('legacy').getStore().req.res.writeHead(403).end('Blocked')
+        const req = getActiveRequest()
+        req.res.writeHead(403).end('Blocked')
         abortController.abort()
       })
 

packages/datadog-instrumentations/test/passport.spec.js

@@ -7,7 +7,7 @@ const sinon = require('sinon')
 
 const axios = require('axios').create({ validateStatus: null })
 const agent = require('../../dd-trace/test/plugins/agent')
-const { storage } = require('../../datadog-core')
+const { getActiveRequest } = require('../../dd-trace/src/appsec/store')
 const { withVersions } = require('../../dd-trace/test/setup/mocha')
 
 const users = [
@@ -149,7 +149,8 @@ withVersions('passport', 'passport', version => {
       const cookie = login.headers['set-cookie'][0]
 
       subscriberStub.callsFake(({ abortController }) => {
-        const res = storage('legacy').getStore().req.res
+        const req = getActiveRequest()
+        const res = req.res
         res.writeHead(403)
         res.constructor.prototype.end.call(res, 'Blocked')
         abortController.abort()

packages/datadog-plugin-http/src/server.js

@@ -2,10 +2,13 @@
 
 const ServerPlugin = require('../../dd-trace/src/plugins/server')
 const { storage } = require('../../datadog-core')
+const { withRequest } = require('../../dd-trace/src/appsec/store')
 const web = require('../../dd-trace/src/plugins/util/web')
 const { incomingHttpRequestStart, incomingHttpRequestEnd } = require('../../dd-trace/src/appsec/channels')
 const { COMPONENT, SVC_SRC_KEY } = require('../../dd-trace/src/constants')
 
+const legacyStorage = storage('legacy')
+
 class HttpServerPlugin extends ServerPlugin {
   static id = 'http'
 
@@ -17,7 +20,7 @@ class HttpServerPlugin extends ServerPlugin {
   }
 
   start ({ req, res, abortController }) {
-    let store = storage('legacy').getStore()
+    let store = legacyStorage.getStore()
     const { name: schemaServiceName, source: schemaServiceSource } = this.serviceName()
     const service = this.config.service || schemaServiceName
     const serviceSource = (this.config.service && service !== this.tracer._service)
@@ -40,15 +43,13 @@ class HttpServerPlugin extends ServerPlugin {
     span._integrationName = this.constructor.id
 
     const context = web.getContext(req)
+    context.parentStore = store
 
-    if (context) {
-      context.parentStore = store
-    }
-
-    // AppSec, IAST, and AI Guard need req/res on the store so downstream
-    // subscribers can access them from the async context.
-    if (incomingHttpRequestStart.hasSubscribers) {
-      store = { ...store, req, res }
+    const appsecActive = incomingHttpRequestStart.hasSubscribers
+    if (appsecActive) {
+      // AppSec, IAST, and AI Guard need req on the store so downstream
+      // subscribers can access them from the async context.
+      store = withRequest(store, req)
     }
 
     this.enter(span, store)
@@ -58,7 +59,7 @@ class HttpServerPlugin extends ServerPlugin {
       context.instrumented = true
     }
 
-    if (incomingHttpRequestStart.hasSubscribers) {
+    if (appsecActive) {
       incomingHttpRequestStart.publish({ req, res, abortController }) // TODO: no need to make a new object here
     }
   }

packages/datadog-plugin-http/test/server.spec.js

@@ -7,6 +7,8 @@ const { afterEach, beforeEach, describe, it } = require('mocha')
 const sinon = require('sinon')
 
 const { incomingHttpRequestStart } = require('../../dd-trace/src/appsec/channels')
+const { storage } = require('../../datadog-core')
+const { getRequest } = require('../../dd-trace/src/appsec/store')
 const agent = require('../../dd-trace/test/plugins/agent')
 const { withNamingSchema } = require('../../dd-trace/test/setup/mocha')
 const { rawExpectedSchema } = require('./naming')
@@ -150,6 +152,28 @@ describe('Plugin', () => {
           axios.get(`http://localhost:${port}/user`).catch(done)
         })
 
+        it('should preserve request access through child scope activation without strong fields', done => {
+          const spy = sinon.spy()
+          incomingHttpRequestStart.subscribe(spy)
+
+          app = (req, res) => {
+            const childSpan = tracer.startSpan('child')
+
+            tracer.scope().activate(childSpan, () => {
+              const store = storage('legacy').getStore()
+
+              sinon.assert.calledOnce(spy)
+              assert.strictEqual(getRequest(store), req)
+              assert.strictEqual(Object.hasOwn(store, 'req'), false)
+              assert.strictEqual(Object.hasOwn(store, 'res'), false)
+            })
+
+            done()
+          }
+
+          axios.get(`http://localhost:${port}/user`).catch(done)
+        })
+
         it('should run the request\'s close event in the correct context', done => {
           app = (req, res) => {
             req.on('close', () => {

packages/dd-trace/src/aiguard/sdk.js

@@ -2,7 +2,7 @@
 
 const rfdc = require('../../../../vendor/dist/rfdc')({ proto: false, circles: false })
 const { HTTP_CLIENT_IP, NETWORK_CLIENT_IP } = require('../../../../ext/tags')
-const { storage } = require('../../../datadog-core')
+const { getActiveRequest } = require('../appsec/store')
 const log = require('../log')
 const { extractIp } = require('../plugins/util/ip_extractor')
 const telemetryMetrics = require('../telemetry/metrics')
@@ -154,7 +154,7 @@ class AIGuard extends NoopAIGuard {
 
     if (!needsHttpClientIp && !needsNetworkClientIp) return
 
-    const req = storage('legacy').getStore()?.req
+    const req = getActiveRequest()
 
     if (!req) return
 

packages/dd-trace/src/appsec/graphql.js

@@ -1,8 +1,8 @@
 'use strict'
 
-const { storage } = require('../../../datadog-core')
 const log = require('../log')
 const web = require('../plugins/util/web')
+const { getActiveRequest } = require('./store')
 const {
   addSpecificEndpoint,
   specificBlockingTypes,
@@ -33,7 +33,7 @@ function disable () {
 }
 
 function onGraphqlStartResolve ({ context, resolverInfo }) {
-  const req = storage('legacy').getStore()?.req
+  const req = getActiveRequest()
 
   if (!req) return
 
@@ -52,7 +52,7 @@ function onGraphqlStartResolve ({ context, resolverInfo }) {
 }
 
 function enterInApolloMiddleware (data) {
-  const req = data?.req || storage('legacy').getStore()?.req
+  const req = data?.req || getActiveRequest()
   if (!req) return
 
   graphqlRequestData.set(req, {
@@ -61,7 +61,7 @@ function enterInApolloMiddleware (data) {
 }
 
 function enterInApolloServerCoreRequest () {
-  const req = storage('legacy').getStore()?.req
+  const req = getActiveRequest()
   if (!req) return
 
   graphqlRequestData.set(req, {
@@ -71,7 +71,7 @@ function enterInApolloServerCoreRequest () {
 }
 
 function enterInApolloRequest () {
-  const req = storage('legacy').getStore()?.req
+  const req = getActiveRequest()
 
   const requestData = graphqlRequestData.get(req)
   if (requestData) {
@@ -83,7 +83,7 @@ function enterInApolloRequest () {
 }
 
 function beforeWriteApolloGraphqlResponse ({ abortController, abortData }) {
-  const req = storage('legacy').getStore()?.req
+  const req = getActiveRequest()
   if (!req) return
 
   const requestData = graphqlRequestData.get(req)

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

@@ -4,7 +4,6 @@ const log = require('../log')
 const web = require('../plugins/util/web')
 const { extractIp } = require('../plugins/util/ip_extractor')
 const { HTTP_CLIENT_IP } = require('../../../../ext/tags')
-const { storage } = require('../../../datadog-core')
 const { IS_SERVERLESS } = require('../serverless')
 const RuleManager = require('./rule_manager')
 const appsecRemoteConfig = require('./remote_config')
@@ -40,6 +39,7 @@ const Reporter = require('./reporter')
 const appsecTelemetry = require('./telemetry')
 const apiSecuritySampler = require('./api_security_sampler')
 const { isBlocked, block, callBlockDelegation, setTemplates, getBlockingAction } = require('./blocking')
+const { getActiveRequest } = require('./store')
 const UserTracking = require('./user_tracking')
 const graphql = require('./graphql')
 const rasp = require('./rasp')
@@ -116,8 +116,7 @@ function onRequestBodyParsed ({ req, res, body, abortController }) {
   if (body === undefined || body === null) return
 
   if (!req) {
-    const store = storage('legacy').getStore()
-    req = store?.req
+    req = getActiveRequest()
   }
 
   const rootSpan = web.root(req)
@@ -258,8 +257,8 @@ function incomingHttpEndTranslator ({ req, res }) {
 }
 
 function onPassportVerify ({ framework, login, user, success, abortController }) {
-  const store = storage('legacy').getStore()
-  const rootSpan = store?.req && web.root(store.req)
+  const req = getActiveRequest()
+  const rootSpan = req && web.root(req)
 
   if (!rootSpan) {
     log.warn('[ASM] No rootSpan found in onPassportVerify')
@@ -268,12 +267,12 @@ function onPassportVerify ({ framework, login, user, success, abortController })
 
   const results = UserTracking.trackLogin(framework, login, user, success, rootSpan)
 
-  handleResults(results?.actions, store.req, store.req.res, rootSpan, abortController)
+  handleResults(results?.actions, req, web.getContext(req)?.res, rootSpan, abortController)
 }
 
 function onPassportDeserializeUser ({ user, abortController }) {
-  const store = storage('legacy').getStore()
-  const rootSpan = store?.req && web.root(store.req)
+  const req = getActiveRequest()
+  const rootSpan = req && web.root(req)
 
   if (!rootSpan) {
     log.warn('[ASM] No rootSpan found in onPassportDeserializeUser')
@@ -282,7 +281,7 @@ function onPassportDeserializeUser ({ user, abortController }) {
 
   const results = UserTracking.trackUser(user, rootSpan)
 
-  handleResults(results?.actions, store.req, store.req.res, rootSpan, abortController)
+  handleResults(results?.actions, req, web.getContext(req)?.res, rootSpan, abortController)
 }
 
 function onExpressSession ({ req, res, sessionId, abortController }) {
@@ -308,8 +307,7 @@ function onRequestQueryParsed ({ req, res, query, abortController }) {
   if (!query || typeof query !== 'object') return
 
   if (!req) {
-    const store = storage('legacy').getStore()
-    req = store?.req
+    req = getActiveRequest()
   }
 
   const rootSpan = web.root(req)

packages/dd-trace/src/appsec/rasp/command_injection.js

@@ -1,8 +1,9 @@
 'use strict'
 
 const { childProcessExecutionTracingChannel } = require('../channels')
-const { storage } = require('../../../../datadog-core')
 const addresses = require('../addresses')
+const web = require('../../plugins/util/web')
+const { getActiveRequest } = require('../store')
 const waf = require('../waf')
 const { RULE_TYPES, handleResult } = require('./utils')
 
@@ -27,8 +28,7 @@ function disable () {
 function analyzeCommandInjection ({ file, fileArgs, shell, abortController }) {
   if (!file) return
 
-  const store = storage('legacy').getStore()
-  const req = store?.req
+  const req = getActiveRequest()
   if (!req) return
 
   const ephemeral = {}
@@ -46,8 +46,7 @@ function analyzeCommandInjection ({ file, fileArgs, shell, abortController }) {
 
   const result = waf.run({ ephemeral }, req, raspRule)
 
-  const res = store?.res
-  handleResult(result, req, res, abortController, config, raspRule)
+  handleResult(result, req, web.getContext(req)?.res, abortController, config, raspRule)
 }
 
 module.exports = {

packages/dd-trace/src/appsec/rasp/lfi.js

@@ -4,7 +4,9 @@ const { isAbsolute } = require('path')
 
 const { fsOperationStart, incomingHttpRequestStart, expressResponseRenderStart } = require('../channels')
 const { storage } = require('../../../../datadog-core')
+const web = require('../../plugins/util/web')
 const { FS_OPERATION_PATH } = require('../addresses')
+const { getRequest } = require('../store')
 const waf = require('../waf')
 const { enable: enableFsPlugin, disable: disableFsPlugin, RASP_MODULE } = require('./fs-plugin')
 const { RULE_TYPES, handleResult } = require('./utils')
@@ -53,16 +55,18 @@ function analyzeLfiInResponseRender (ctx) {
   const store = storage('legacy').getStore()
   if (!store) return
 
-  analyzeLfiPath(ctx.view, ctx.req, store.res, ctx.abortController)
+  analyzeLfiPath(ctx.view, ctx.req, web.getContext(ctx.req)?.res, ctx.abortController)
 }
 
 function analyzeLfi (ctx) {
   const store = storage('legacy').getStore()
-  if (!store) return
+  const fs = store?.fs
+  if (!fs) return
 
-  const { req, fs, res } = store
-  if (!req || !fs) return
+  const req = getRequest(store)
+  if (!req) return
 
+  const res = web.getContext(req)?.res
   for (const path of getPaths(ctx, fs)) {
     analyzeLfiPath(path, req, res, ctx.abortController)
   }