Disable appsec if missing or malformed user custom rules file

.github/actions/check-release-branch/action.yml

@@ -22,4 +22,8 @@ runs:
       shell: bash
       run: |
         git checkout ${{ inputs.release-branch }}
-        git cherry-pick ${{ steps.get-commit-to-cherry-pick.outputs.commit-to-cherry-pick}}
+        {
+          git cherry-pick ${{ steps.get-commit-to-cherry-pick.outputs.commit-to-cherry-pick}}
+        } || {
+          git status ; git diff ; exit 1
+        }

.github/workflows/project.yml

@@ -14,6 +14,9 @@ concurrency:
 jobs:
   integration:
     strategy:
+      # when one version fails, say 14, all the other versions are stopped
+      # setting fail-fast to false in an attempt to prevent this from happening
+      fail-fast: false
       matrix:
         version: [14, 16, 18, latest]
     runs-on: ubuntu-latest

README.md

@@ -202,7 +202,7 @@ For these reasons it's necessary to have custom-built bundler plugins. Such plug
 
 ### Esbuild Support
 
-This library provides experimental esbuild support in the form of an esbuild plugin, and requires at least Node.js v14.17. To use the plugin, make sure you have `dd-trace@3+` installed, and then require the `dd-trace/esbuild` module when building your bundle.
+This library provides experimental esbuild support in the form of an esbuild plugin, and currently requires at least Node.js v16.17 or v18.7. To use the plugin, make sure you have `dd-trace@3+` installed, and then require the `dd-trace/esbuild` module when building your bundle.
 
 Here's an example of how one might use `dd-trace` with esbuild:
 

integration-tests/esbuild/basic-test.js

@@ -1,5 +1,11 @@
 #!/usr/bin/env node
 
+// TODO: add support for Node.js v14.17+ and v16.0+
+if (Number(process.versions.node.split('.')[0]) < 16) {
+  console.error(`Skip esbuild test for node@${process.version}`) // eslint-disable-line no-console
+  process.exit(0)
+}
+
 const tracer = require('../../').init() // dd-trace
 
 const assert = require('assert')
@@ -21,7 +27,7 @@ app.get('/', async (_req, res) => {
   assert.equal(
     tracer.scope().active().context()._tags.component,
     'express',
-    'the sample app bundled by esbuild is not properly instrumented'
+    `the sample app bundled by esbuild is not properly instrumented. using node@${process.version}`
   ) // bad exit
 
   res.json({ narwhal: 'bacons' })

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

@@ -21,8 +21,8 @@ function enable (_config) {
   try {
     setTemplates(_config)
 
-    const rules = _config.appsec.rules || require('./recommended.json')
-    enableFromRules(_config, rules)
+    // TODO: inline this function
+    enableFromRules(_config, _config.appsec.rules)
   } catch (err) {
     abortEnable(err)
   }

packages/dd-trace/src/config.js

@@ -300,8 +300,8 @@ class Config {
     )
 
     const DD_APPSEC_RULES = coalesce(
-      safeJsonParse(maybeFile(appsec.rules)),
-      safeJsonParse(maybeFile(process.env.DD_APPSEC_RULES))
+      appsec.rules,
+      process.env.DD_APPSEC_RULES
     )
     const DD_APPSEC_TRACE_RATE_LIMIT = coalesce(
       parseInt(appsec.rateLimit),
@@ -479,7 +479,7 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)
     this.tagsHeaderMaxLength = parseInt(DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH)
     this.appsec = {
       enabled: DD_APPSEC_ENABLED,
-      rules: DD_APPSEC_RULES,
+      rules: DD_APPSEC_RULES ? safeJsonParse(maybeFile(DD_APPSEC_RULES)) : require('./appsec/recommended.json'),
       rateLimit: DD_APPSEC_TRACE_RATE_LIMIT,
       wafTimeout: DD_APPSEC_WAF_TIMEOUT,
       obfuscatorKeyRegex: DD_APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP,

packages/dd-trace/test/appsec/index.spec.js

@@ -15,7 +15,6 @@ const axios = require('axios')
 const getPort = require('get-port')
 
 const blockedTemplate = require('../../src/appsec/blocked_templates')
-const recommendedJson = require('../../src/appsec/recommended.json')
 
 describe('AppSec Index', () => {
   let config
@@ -102,13 +101,6 @@ describe('AppSec Index', () => {
       expect(incomingHttpRequestEnd.subscribe).to.not.have.been.called
       expect(Gateway.manager.addresses).to.be.empty
     })
-
-    it('should load recommended.json rules if not provided by the user', () => {
-      config.appsec.rules = undefined
-      AppSec.enable(config)
-
-      expect(RuleManager.applyRules).to.have.been.calledOnceWithExactly(recommendedJson, config.appsec)
-    })
   })
 
   describe('disable', () => {

packages/dd-trace/test/config.spec.js

@@ -16,6 +16,8 @@ describe('Config', () => {
   let existsSyncReturn
   let osType
 
+  const RECOMMENDED_JSON_PATH = require.resolve('../src/appsec/recommended.json')
+  const RECOMMENDED_JSON = require(RECOMMENDED_JSON_PATH)
   const RULES_JSON_PATH = require.resolve('./fixtures/config/appsec-rules.json')
   const RULES_JSON = require(RULES_JSON_PATH)
   const BLOCKED_TEMPLATE_HTML_PATH = require.resolve('./fixtures/config/appsec-blocked-template.html')
@@ -95,7 +97,7 @@ describe('Config', () => {
     expect(config).to.have.nested.property('experimental.exporter', undefined)
     expect(config).to.have.nested.property('experimental.enableGetRumData', false)
     expect(config).to.have.nested.property('appsec.enabled', undefined)
-    expect(config).to.have.nested.property('appsec.rules', undefined)
+    expect(config).to.have.nested.property('appsec.rules', RECOMMENDED_JSON)
     expect(config).to.have.nested.property('appsec.rateLimit', 100)
     expect(config).to.have.nested.property('appsec.wafTimeout', 5e3)
     expect(config).to.have.nested.property('appsec.obfuscatorKeyRegex').with.length(155)
@@ -533,7 +535,7 @@ describe('Config', () => {
     process.env.DD_TRACE_EXPERIMENTAL_GET_RUM_DATA_ENABLED = 'true'
     process.env.DD_TRACE_EXPERIMENTAL_INTERNAL_ERRORS_ENABLED = 'true'
     process.env.DD_APPSEC_ENABLED = 'false'
-    process.env.DD_APPSEC_RULES = require.resolve('../src/appsec/recommended.json')
+    process.env.DD_APPSEC_RULES = RECOMMENDED_JSON_PATH
     process.env.DD_APPSEC_TRACE_RATE_LIMIT = 11
     process.env.DD_APPSEC_WAF_TIMEOUT = 11
     process.env.DD_APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP = '^$'
@@ -666,6 +668,32 @@ describe('Config', () => {
       }
     })
 
+    expect(config).to.have.deep.property('appsec', {
+      enabled: true,
+      rules: RECOMMENDED_JSON,
+      rateLimit: 42,
+      wafTimeout: 42,
+      obfuscatorKeyRegex: '.*',
+      obfuscatorValueRegex: '.*',
+      blockedTemplateHtml: undefined,
+      blockedTemplateJson: undefined
+    })
+  })
+
+  it('should left undefined appsec rules if user rules file could not be loaded', () => {
+    const config = new Config({
+      appsec: {
+        enabled: true,
+        rules: '/not/existing/path/or/bad/format.json',
+        rateLimit: 42,
+        wafTimeout: 42,
+        obfuscatorKeyRegex: '.*',
+        obfuscatorValueRegex: '.*',
+        blockedTemplateHtml: undefined,
+        blockedTemplateJson: undefined
+      }
+    })
+
     expect(config).to.have.deep.property('appsec', {
       enabled: true,
       rules: undefined,