From ccc741a5683d48c922c5d9023755fbd2faa6558f Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Tue, 22 Apr 2025 16:45:21 +0200 Subject: [PATCH 01/22] WIP --- package.json | 7 +- packages/dd-trace/src/appsec/rule_manager.js | 64 ++++++++++++++++ packages/dd-trace/src/appsec/waf/index.js | 1 + .../dd-trace/src/appsec/waf/waf_manager.js | 74 ++++++++++++++++++- packages/dd-trace/src/remote_config/index.js | 7 +- .../dd-trace/src/remote_config/manager.js | 4 + 6 files changed, 151 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index f7c0ad4d310..b2954668668 100644 --- a/package.json +++ b/package.json @@ -84,7 +84,7 @@ }, "dependencies": { "@datadog/libdatadog": "^0.5.1", - "@datadog/native-appsec": "8.5.2", + "@datadog/native-appsec": "file:datadog-native-appsec-8.5.2.tgz", "@datadog/native-iast-taint-tracking": "4.0.0", "@datadog/native-metrics": "^3.1.1", "@datadog/pprof": "5.8.0", @@ -153,5 +153,8 @@ "tap": "^16.3.10", "tiktoken": "^1.0.21", "yaml": "^2.8.0" - } + }, + "bundledDependencies": [ + "@datadog/native-appsec" + ] } diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 10a66755d49..9937e352b4d 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -26,6 +26,67 @@ function loadRules (config) { } function updateWafFromRC ({ toUnapply, toApply, toModify }) { + console.log('\n\nupdateWafFromRC') + //console.log('toUnapply', toUnapply) + //console.log('toApply', toApply) + //console.log('toModify', toModify) + + + for (const item of toUnapply) { + // TODO Maybe item.product.startsWith('ASM') ?? or setting product handler properly + console.log('toUnapply', item.product, item.path) + if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue + + waf.wafManager.remove(item.path) + console.log(item.path, item.apply_state, item.apply_error) + } + + // TODO refactor and add try catch + for (const item of toApply) { + try { + // TODO Maybe item.product.startsWith('ASM') ?? or setting product handler properly + console.log('toApply', item.product, item.path) + if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue + + let updateResult + if (item.product === 'ASM_DD') { + updateResult = waf.wafManager.updateASMDD(item.file, item.path) + } else { + updateResult = waf.wafManager.update(item.file, item.path) + } + + if (updateResult.success) { + item.apply_state = ACKNOWLEDGED + } else { + item.apply_state = ERROR + item.apply_error = updateResult.error + } + console.log(item.path, item.apply_state, item.apply_error) + } catch (e) { + console.log(e) + } + } + + for (const item of toModify) { + try { + // TODO Maybe item.product.startsWith('ASM') ?? or setting product handler properly + console.log('toModify', item.product, item.path) + if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue + + const updateResult = waf.wafManager.update(item.file, item.path) + if (updateResult.success) { + item.apply_state = ACKNOWLEDGED + } else { + item.apply_state = ERROR + item.apply_error = updateResult.error + } + console.log(item.path, item.apply_state, item.apply_error) + } catch (e) { + console.log(e) + } + } + + /* const batch = new Set() const newRulesData = new SpyMap(appliedRulesData) @@ -155,9 +216,12 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { } for (const config of batch) { + console.log('@@ Rule Manager applying states before', config.apply_state, config.apply_error) config.apply_state = newApplyState if (newApplyError) config.apply_error = newApplyError + console.log('@@ Rule Manager applying states', config.apply_state, config.apply_error) } + */ } // A Map with a new prop `modified`, a bool that indicates if the Map was modified diff --git a/packages/dd-trace/src/appsec/waf/index.js b/packages/dd-trace/src/appsec/waf/index.js index b025a123f46..94095acb148 100644 --- a/packages/dd-trace/src/appsec/waf/index.js +++ b/packages/dd-trace/src/appsec/waf/index.js @@ -35,6 +35,7 @@ function destroy () { } function update (newRules) { + console.log('call update with'/*, newRules*/) // TODO: check race conditions between Appsec enable/disable and WAF updates, the whole RC state management in general if (!waf.wafManager) throw new Error('Cannot update disabled WAF') diff --git a/packages/dd-trace/src/appsec/waf/waf_manager.js b/packages/dd-trace/src/appsec/waf/waf_manager.js index e745818aa9e..6f6b73bcfc6 100644 --- a/packages/dd-trace/src/appsec/waf/waf_manager.js +++ b/packages/dd-trace/src/appsec/waf/waf_manager.js @@ -6,12 +6,15 @@ const WAFContextWrapper = require('./waf_context_wrapper') const contexts = new WeakMap() +const DEFAULT_WAF_CONFIG_PATH = 'datadog/00/ASM_DD/default/config' + class WAFManager { constructor (rules, config) { this.config = config this.wafTimeout = config.wafTimeout this.ddwaf = this._loadDDWAF(rules) this.rulesVersion = this.ddwaf.diagnostics.ruleset_version + this.defaultRules = rules Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules, true) } @@ -23,8 +26,9 @@ class WAFManager { this.ddwafVersion = DDWAF.version() const { obfuscatorKeyRegex, obfuscatorValueRegex } = this.config - return new DDWAF(rules, { obfuscatorKeyRegex, obfuscatorValueRegex }) + return new DDWAF(rules, DEFAULT_WAF_CONFIG_PATH, { obfuscatorKeyRegex, obfuscatorValueRegex }) } catch (err) { + console.log('what happened', err) this.ddwafVersion = this.ddwafVersion || 'unknown' Reporter.reportWafInit(this.ddwafVersion, 'unknown') @@ -51,6 +55,7 @@ class WAFManager { return wafContext } + /* update (newRules) { try { this.ddwaf.update(newRules) @@ -66,6 +71,73 @@ class WAFManager { throw error } } + */ + + updateASMDD(config, path) { + console.log(this.ddwaf.configPaths) + if (this.ddwaf.configPaths.includes(DEFAULT_WAF_CONFIG_PATH)) { + console.log('Removing default config') + this.ddwaf.removeConfig(DEFAULT_WAF_CONFIG_PATH) + } + + const success = this.ddwaf.createOrUpdateConfig(config, path) + + if (!this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { + console.log('Reverting default rules') + this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) + } + + if (this.ddwaf.diagnostics.ruleset_version) { + this.rulesVersion = this.ddwaf.diagnostics.ruleset_version + } + + Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, success) + return { success } + } + + update (rules, path) { + try { + console.log('removeConfig path', path) + this.ddwaf.removeConfig(path) + console.log('createOrUpdateConfig path', path) + const success = this.ddwaf.createOrUpdateConfig(rules, path) + + if (this.ddwaf.diagnostics.ruleset_version) { + this.rulesVersion = this.ddwaf.diagnostics.ruleset_version + } + + Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, success) + return { success } + } catch (error) { + Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) + + throw error + } + + } + + remove (path) { + try { + console.log('removeConfig path', path) + this.ddwaf.removeConfig(path) + + if (!this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { + console.log('Reverting default rules') + this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) + } + + if (this.ddwaf.diagnostics.ruleset_version) { + this.rulesVersion = this.ddwaf.diagnostics.ruleset_version + } + + Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, true) + } catch (error) { + Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) + + throw error + } + } + destroy () { if (this.ddwaf) { diff --git a/packages/dd-trace/src/remote_config/index.js b/packages/dd-trace/src/remote_config/index.js index ab149bdf000..55702586227 100644 --- a/packages/dd-trace/src/remote_config/index.js +++ b/packages/dd-trace/src/remote_config/index.js @@ -6,6 +6,7 @@ const RemoteConfigManager = require('./manager') const RemoteConfigCapabilities = require('./capabilities') const { setCollectionMode } = require('../appsec/user_tracking') const log = require('../log') +const RuleManager = require("../appsec/rule_manager"); let rc @@ -135,9 +136,9 @@ function disableWafUpdate () { rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_SHI, false) rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_CMDI, false) - rc.removeProductHandler('ASM_DATA') - rc.removeProductHandler('ASM_DD') - rc.removeProductHandler('ASM') + //rc.removeProductHandler('ASM_DATA') + //rc.removeProductHandler('ASM_DD') + //rc.removeProductHandler('ASM') rc.off(RemoteConfigManager.kPreUpdate, RuleManager.updateWafFromRC) } diff --git a/packages/dd-trace/src/remote_config/manager.js b/packages/dd-trace/src/remote_config/manager.js index a95f281fb86..dbff5a96c0e 100644 --- a/packages/dd-trace/src/remote_config/manager.js +++ b/packages/dd-trace/src/remote_config/manager.js @@ -270,6 +270,7 @@ class RemoteConfigManager extends EventEmitter { for (const item of list) { // TODO: we need a way to tell if unapply configs were handled by kPreUpdate or not, because they're always // emitted unlike the apply and modify configs + console.log('\n\n@@ Dispatching item - before', item.apply_state, item.apply_error) this._callHandlerFor(action, item) @@ -278,6 +279,7 @@ class RemoteConfigManager extends EventEmitter { } else { this.appliedConfigs.set(item.path, item) } + console.log('@@ Dispatching item - after', item.apply_state, item.apply_error) } } @@ -305,7 +307,9 @@ class RemoteConfigManager extends EventEmitter { // If the handler doesn't accept an `ack` callback, assume `apply_state` is `ACKNOWLEDGED`, // unless it returns a promise, in which case we wait for the promise to be resolved or rejected. // TODO: do we want to pass old and new config ? + console.log('@@ _callHandlerFor before handler', item.apply_state, item.apply_error) const result = handler(action, item.file, item.id) + console.log('@@ _callHandlerFor after handler', item.apply_state, item.apply_error) if (result instanceof Promise) { result.then( () => { item.apply_state = ACKNOWLEDGED }, From f9b42429a34fce8d3c156bf5429ab100afb39df7 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Thu, 24 Apr 2025 17:03:34 +0200 Subject: [PATCH 02/22] Clean up + apply_error --- packages/dd-trace/src/appsec/rule_manager.js | 266 ++---------------- .../dd-trace/src/appsec/waf/waf_manager.js | 66 ++--- 2 files changed, 41 insertions(+), 291 deletions(-) diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 9937e352b4d..1194a87c12a 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -6,17 +6,10 @@ const { ACKNOWLEDGED, ERROR } = require('../remote_config/apply_states') const blocking = require('./blocking') -let defaultRules - -let appliedRulesData = new Map() -let appliedRulesetId -let appliedRulesOverride = new Map() -let appliedExclusions = new Map() -let appliedCustomRules = new Map() let appliedActions = new Map() function loadRules (config) { - defaultRules = config.rules + const defaultRules = config.rules ? JSON.parse(fs.readFileSync(config.rules)) : require('./recommended.json') @@ -26,202 +19,45 @@ function loadRules (config) { } function updateWafFromRC ({ toUnapply, toApply, toModify }) { - console.log('\n\nupdateWafFromRC') - //console.log('toUnapply', toUnapply) - //console.log('toApply', toApply) - //console.log('toModify', toModify) - + const newActions = new SpyMap(appliedActions) for (const item of toUnapply) { - // TODO Maybe item.product.startsWith('ASM') ?? or setting product handler properly - console.log('toUnapply', item.product, item.path) if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue - waf.wafManager.remove(item.path) - console.log(item.path, item.apply_state, item.apply_error) - } - - // TODO refactor and add try catch - for (const item of toApply) { try { - // TODO Maybe item.product.startsWith('ASM') ?? or setting product handler properly - console.log('toApply', item.product, item.path) - if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue - - let updateResult - if (item.product === 'ASM_DD') { - updateResult = waf.wafManager.updateASMDD(item.file, item.path) - } else { - updateResult = waf.wafManager.update(item.file, item.path) - } - - if (updateResult.success) { - item.apply_state = ACKNOWLEDGED - } else { - item.apply_state = ERROR - item.apply_error = updateResult.error - } - console.log(item.path, item.apply_state, item.apply_error) + waf.wafManager.remove(item.path) } catch (e) { - console.log(e) - } - } - - for (const item of toModify) { - try { - // TODO Maybe item.product.startsWith('ASM') ?? or setting product handler properly - console.log('toModify', item.product, item.path) - if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue - - const updateResult = waf.wafManager.update(item.file, item.path) - if (updateResult.success) { - item.apply_state = ACKNOWLEDGED - } else { - item.apply_state = ERROR - item.apply_error = updateResult.error - } - console.log(item.path, item.apply_state, item.apply_error) - } catch (e) { - console.log(e) - } - } - - /* - const batch = new Set() - - const newRulesData = new SpyMap(appliedRulesData) - let newRuleset - let newRulesetId - const newRulesOverride = new SpyMap(appliedRulesOverride) - const newExclusions = new SpyMap(appliedExclusions) - const newCustomRules = new SpyMap(appliedCustomRules) - const newActions = new SpyMap(appliedActions) - - for (const item of toUnapply) { - const { product, id } = item - - if (product === 'ASM_DATA') { - newRulesData.delete(id) - } else if (product === 'ASM_DD') { - if (appliedRulesetId === id) { - newRuleset = defaultRules - } - } else if (product === 'ASM') { - newRulesOverride.delete(id) - newExclusions.delete(id) - newCustomRules.delete(id) - newActions.delete(id) + item.apply_state = ERROR + item.apply_error = e.toString() } } for (const item of [...toApply, ...toModify]) { - const { product, id, file } = item - - if (product === 'ASM_DATA') { - if (file && file.rules_data && file.rules_data.length) { - newRulesData.set(id, file.rules_data) - } - - batch.add(item) - } else if (product === 'ASM_DD') { - if (appliedRulesetId && appliedRulesetId !== id && newRuleset !== defaultRules) { - item.apply_state = ERROR - item.apply_error = 'Multiple ruleset received in ASM_DD' - } else { - if (file?.rules?.length) { - const { version, metadata, rules, processors, scanners } = file - - newRuleset = { version, metadata, rules, processors, scanners } - newRulesetId = id - } - - batch.add(item) - } - } else if (product === 'ASM') { - if (file?.rules_override?.length) { - newRulesOverride.set(id, file.rules_override) - } - - if (file?.exclusions?.length) { - newExclusions.set(id, file.exclusions) - } - - if (file?.custom_rules?.length) { - newCustomRules.set(id, file.custom_rules) - } - - if (file?.actions?.length) { - newActions.set(id, file.actions) - } - - batch.add(item) - } - } - - let newApplyState = ACKNOWLEDGED - let newApplyError - - if (newRulesData.modified || - newRuleset || - newRulesOverride.modified || - newExclusions.modified || - newCustomRules.modified || - newActions.modified - ) { - const payload = newRuleset || {} - - if (newRulesData.modified) { - payload.rules_data = mergeRulesData(newRulesData) - } - if (newRulesOverride.modified) { - payload.rules_override = concatArrays(newRulesOverride) - } - if (newExclusions.modified) { - payload.exclusions = concatArrays(newExclusions) - } - if (newCustomRules.modified) { - payload.custom_rules = concatArrays(newCustomRules) - } - if (newActions.modified) { - payload.actions = concatArrays(newActions) - } + if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue try { - waf.update(payload) - - if (newRulesData.modified) { - appliedRulesData = newRulesData - } - if (newRuleset) { - appliedRulesetId = newRulesetId - } - if (newRulesOverride.modified) { - appliedRulesOverride = newRulesOverride - } - if (newExclusions.modified) { - appliedExclusions = newExclusions - } - if (newCustomRules.modified) { - appliedCustomRules = newCustomRules + const updateResult = waf.wafManager.update(item.product, item.file, item.path) + item.apply_state = updateResult.success ? ACKNOWLEDGED : ERROR + item.apply_error = updateResult.error + + // check asm actions + if (updateResult.success && item.product === 'ASM') { + if (item.file?.actions?.length) { + newActions.set(item.id, item.file.actions) + } } - if (newActions.modified) { - appliedActions = newActions - blocking.setDefaultBlockingActionParameters(concatArrays(newActions)) - } - } catch (err) { - newApplyState = ERROR - newApplyError = err.toString() + } catch (e) { + item.apply_state = ERROR + item.apply_error = e.toString() } } - for (const config of batch) { - console.log('@@ Rule Manager applying states before', config.apply_state, config.apply_error) - config.apply_state = newApplyState - if (newApplyError) config.apply_error = newApplyError - console.log('@@ Rule Manager applying states', config.apply_state, config.apply_error) + // Manage blocking actions + if (newActions.modified) { + appliedActions = newActions + blocking.setDefaultBlockingActionParameters(concatArrays(newActions)) } - */ } // A Map with a new prop `modified`, a bool that indicates if the Map was modified @@ -252,67 +88,9 @@ function concatArrays (files) { return [...files.values()].flat() } -/* - ASM_DATA Merge strategy: - The merge should be based on the id and type. For any duplicate items, the longer expiration should be taken. - As a result, multiple Rule Data may use the same DATA_ID and DATA_TYPE. In this case, all values are considered part - of a set and are merged. For instance, a denylist customized by environment may use a global Rule Data for all - environments and a Rule Data per environment -*/ - -function mergeRulesData (files) { - const mergedRulesData = new Map() - for (const [, file] of files) { - for (const ruleData of file) { - const key = `${ruleData.id}+${ruleData.type}` - if (mergedRulesData.has(key)) { - const existingRulesData = mergedRulesData.get(key) - ruleData.data.reduce(rulesReducer, existingRulesData.data) - } else { - mergedRulesData.set(key, copyRulesData(ruleData)) - } - } - } - return [...mergedRulesData.values()] -} - -function rulesReducer (existingEntries, rulesDataEntry) { - const existingEntry = existingEntries.find((entry) => entry.value === rulesDataEntry.value) - if (existingEntry && !('expiration' in existingEntry)) return existingEntries - if (existingEntry && 'expiration' in rulesDataEntry && rulesDataEntry.expiration > existingEntry.expiration) { - existingEntry.expiration = rulesDataEntry.expiration - } else if (existingEntry && !('expiration' in rulesDataEntry)) { - delete existingEntry.expiration - } else if (!existingEntry) { - existingEntries.push({ ...rulesDataEntry }) - } - return existingEntries -} - -function copyRulesData (rulesData) { - const copy = { ...rulesData } - if (copy.data) { - const data = [] - copy.data.forEach(item => { - data.push({ ...item }) - }) - copy.data = data - } - return copy -} - function clearAllRules () { waf.destroy() - - defaultRules = undefined - - appliedRulesData.clear() - appliedRulesetId = undefined - appliedRulesOverride.clear() - appliedExclusions.clear() - appliedCustomRules.clear() appliedActions.clear() - blocking.setDefaultBlockingActionParameters(undefined) } diff --git a/packages/dd-trace/src/appsec/waf/waf_manager.js b/packages/dd-trace/src/appsec/waf/waf_manager.js index 6f6b73bcfc6..60700033f74 100644 --- a/packages/dd-trace/src/appsec/waf/waf_manager.js +++ b/packages/dd-trace/src/appsec/waf/waf_manager.js @@ -7,6 +7,10 @@ const WAFContextWrapper = require('./waf_context_wrapper') const contexts = new WeakMap() const DEFAULT_WAF_CONFIG_PATH = 'datadog/00/ASM_DD/default/config' +const DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR = [ + "error", "errors", + "exclusions", "rules", "processors", "rules_override", "rules_data", "custom_rules", "actions", "scanners" +] class WAFManager { constructor (rules, config) { @@ -19,6 +23,10 @@ class WAFManager { Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules, true) } + static computeUpdateWafError (diagnostics) { + return JSON.stringify(diagnostics, DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR) + } + _loadDDWAF (rules) { try { // require in `try/catch` because this can throw at require time @@ -28,7 +36,6 @@ class WAFManager { const { obfuscatorKeyRegex, obfuscatorValueRegex } = this.config return new DDWAF(rules, DEFAULT_WAF_CONFIG_PATH, { obfuscatorKeyRegex, obfuscatorValueRegex }) } catch (err) { - console.log('what happened', err) this.ddwafVersion = this.ddwafVersion || 'unknown' Reporter.reportWafInit(this.ddwafVersion, 'unknown') @@ -55,59 +62,26 @@ class WAFManager { return wafContext } - /* - update (newRules) { + update (product, rules, path) { try { - this.ddwaf.update(newRules) - - if (this.ddwaf.diagnostics.ruleset_version) { - this.rulesVersion = this.ddwaf.diagnostics.ruleset_version + if (product === 'ASM_DD' && this.ddwaf.configPaths.includes(DEFAULT_WAF_CONFIG_PATH)) { + this.ddwaf.removeConfig(DEFAULT_WAF_CONFIG_PATH) } - Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, true) - } catch (error) { - Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) - - throw error - } - } - */ - - updateASMDD(config, path) { - console.log(this.ddwaf.configPaths) - if (this.ddwaf.configPaths.includes(DEFAULT_WAF_CONFIG_PATH)) { - console.log('Removing default config') - this.ddwaf.removeConfig(DEFAULT_WAF_CONFIG_PATH) - } - - const success = this.ddwaf.createOrUpdateConfig(config, path) - - if (!this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { - console.log('Reverting default rules') - this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) - } - - if (this.ddwaf.diagnostics.ruleset_version) { - this.rulesVersion = this.ddwaf.diagnostics.ruleset_version - } - - Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, success) - return { success } - } - - update (rules, path) { - try { - console.log('removeConfig path', path) - this.ddwaf.removeConfig(path) - console.log('createOrUpdateConfig path', path) const success = this.ddwaf.createOrUpdateConfig(rules, path) + const updateDiagnostics = this.ddwaf.diagnostics - if (this.ddwaf.diagnostics.ruleset_version) { + if (updateDiagnostics.ruleset_version) { this.rulesVersion = this.ddwaf.diagnostics.ruleset_version } Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, success) - return { success } + + if (product === 'ASM_DD' && !success && !this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { + this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) + } + + return { success, error: !success ? WAFManager.computeUpdateWafError(updateDiagnostics) : undefined } } catch (error) { Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) @@ -118,7 +92,6 @@ class WAFManager { remove (path) { try { - console.log('removeConfig path', path) this.ddwaf.removeConfig(path) if (!this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { @@ -138,7 +111,6 @@ class WAFManager { } } - destroy () { if (this.ddwaf) { this.ddwaf.dispose() From cf6b37ca4dd3b8693e5be0fbf8eb85b71a5175f5 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Wed, 30 Apr 2025 13:06:50 +0200 Subject: [PATCH 03/22] WAF update diagnostics propagation through RC and telemetry --- packages/dd-trace/src/appsec/reporter.js | 54 ++++++++++++++++++- packages/dd-trace/src/appsec/rule_manager.js | 13 ++++- .../dd-trace/src/appsec/waf/waf_manager.js | 16 ++---- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index a6d7368bf22..1194a4e2af4 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -1,5 +1,8 @@ 'use strict' +const dc = require('dc-polyfill') +const zlib = require('zlib') + const Limiter = require('../rate_limiter') const { storage } = require('../../../datadog-core') const web = require('../plugins/util/web') @@ -14,7 +17,6 @@ const { updateRateLimitedMetric, getRequestMetrics } = require('./telemetry') -const zlib = require('zlib') const { keepTrace } = require('../priority_sampler') const { ASM } = require('../standalone/product') @@ -25,6 +27,20 @@ const COLLECTED_REQUEST_BODY_MAX_STRING_LENGTH = 4096 const COLLECTED_REQUEST_BODY_MAX_DEPTH = 20 const COLLECTED_REQUEST_BODY_MAX_ELEMENTS_PER_NODE = 256 +const telemetryLogCh = dc.channel('datadog:telemetry:log') + +const WAF_DIAGNOSTICS_CONFIG_KEYS_TO_REPORT = [ + 'rules', + 'custom_rules', + 'exclusions', + 'actions', + 'processors', + 'scanners', + 'rules_override', + 'rules_data', + 'exclusion_data' +] + // default limiter, configurable with setRateLimit() let limiter = new Limiter(100) @@ -225,6 +241,41 @@ function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}, success incrementWafInitMetric(wafVersion, rulesVersion, success) } +function reportSuccessfulWafUpdate (product, rcConfigId, diagnostics) { + for (const configKey of WAF_DIAGNOSTICS_CONFIG_KEYS_TO_REPORT) { + if (!diagnostics[configKey]) continue + + if (diagnostics[configKey].error) { + telemetryLogCh.publish({ + message: diagnostics[configKey].error, + level: 'ERROR', + tags: `log_type:rc::${product}::diagnostic, appsec_config_key:${configKey}, rc_config_id:${rcConfigId}` + }) + continue + } + + if (diagnostics[configKey].errors) { + for (const [errorMessage, errorIds] of Object.entries(diagnostics[configKey].errors)) { + telemetryLogCh.publish({ + message: `"${errorMessage}": ${JSON.stringify(errorIds)}`, + level: 'ERROR', + tags: `log_type:rc::${product}::diagnostic, appsec_config_key:${configKey}, rc_config_id:${rcConfigId}` + }) + } + } + + if (diagnostics[configKey].warnings) { + for (const [warningMessage, warningIds] of Object.entries(diagnostics[configKey].warnings)) { + telemetryLogCh.publish({ + message: `"${warningMessage}": ${JSON.stringify(warningIds)}`, + level: 'ERROR', + tags: `log_type:rc::${product}::diagnostic, appsec_config_key:${configKey}, rc_config_id:${rcConfigId}` + }) + } + } + } +} + function reportMetrics (metrics, raspRule) { const store = storage('legacy').getStore() const rootSpan = store?.req && web.root(store.req) @@ -485,6 +536,7 @@ module.exports = { filterExtendedHeaders, formatHeaderName, reportWafInit, + reportSuccessfulWafUpdate, reportMetrics, reportAttack, reportWafUpdate: incrementWafUpdatesMetric, diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 1194a87c12a..c8594770831 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -3,9 +3,15 @@ const fs = require('fs') const waf = require('./waf') const { ACKNOWLEDGED, ERROR } = require('../remote_config/apply_states') +const Reporter = require('./reporter') const blocking = require('./blocking') +const DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR = [ + "error", "errors", + "exclusions", "rules", "processors", "rules_override", "rules_data", "custom_rules", "actions", "scanners" +] + let appliedActions = new Map() function loadRules (config) { @@ -38,7 +44,12 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { try { const updateResult = waf.wafManager.update(item.product, item.file, item.path) item.apply_state = updateResult.success ? ACKNOWLEDGED : ERROR - item.apply_error = updateResult.error + + if (updateResult.success) { + Reporter.reportSuccessfulWafUpdate(item.product, item.id, updateResult.diagnostics) + } else { + item.apply_error = JSON.stringify(updateResult.diagnostics, DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR) + } // check asm actions if (updateResult.success && item.product === 'ASM') { diff --git a/packages/dd-trace/src/appsec/waf/waf_manager.js b/packages/dd-trace/src/appsec/waf/waf_manager.js index 60700033f74..38bb5c0d0b0 100644 --- a/packages/dd-trace/src/appsec/waf/waf_manager.js +++ b/packages/dd-trace/src/appsec/waf/waf_manager.js @@ -7,10 +7,6 @@ const WAFContextWrapper = require('./waf_context_wrapper') const contexts = new WeakMap() const DEFAULT_WAF_CONFIG_PATH = 'datadog/00/ASM_DD/default/config' -const DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR = [ - "error", "errors", - "exclusions", "rules", "processors", "rules_override", "rules_data", "custom_rules", "actions", "scanners" -] class WAFManager { constructor (rules, config) { @@ -23,10 +19,6 @@ class WAFManager { Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules, true) } - static computeUpdateWafError (diagnostics) { - return JSON.stringify(diagnostics, DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR) - } - _loadDDWAF (rules) { try { // require in `try/catch` because this can throw at require time @@ -69,10 +61,10 @@ class WAFManager { } const success = this.ddwaf.createOrUpdateConfig(rules, path) - const updateDiagnostics = this.ddwaf.diagnostics + const diagnostics = this.ddwaf.diagnostics - if (updateDiagnostics.ruleset_version) { - this.rulesVersion = this.ddwaf.diagnostics.ruleset_version + if (diagnostics.ruleset_version) { + this.rulesVersion = diagnostics.ruleset_version } Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, success) @@ -81,7 +73,7 @@ class WAFManager { this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) } - return { success, error: !success ? WAFManager.computeUpdateWafError(updateDiagnostics) : undefined } + return { success, diagnostics } } catch (error) { Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) From bf584c8209ff6f0884daa28e2dc87ecb54f601a4 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 2 May 2025 14:45:24 +0200 Subject: [PATCH 04/22] Change metrics for waf update and config errors --- packages/dd-trace/src/appsec/reporter.js | 2 + packages/dd-trace/src/appsec/rule_manager.js | 11 ++++ .../dd-trace/src/appsec/telemetry/index.js | 12 ++++- packages/dd-trace/src/appsec/telemetry/waf.js | 10 ++-- .../dd-trace/src/appsec/waf/waf_manager.js | 52 ++++++------------- 5 files changed, 46 insertions(+), 41 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 1194a4e2af4..84fa068b138 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -10,6 +10,7 @@ const { ipHeaderList } = require('../plugins/util/ip_extractor') const { incrementWafInitMetric, incrementWafUpdatesMetric, + incrementWafConfigErrorsMetric, incrementWafRequestsMetric, updateWafRequestsMetricTags, updateRaspRequestsMetricTags, @@ -540,6 +541,7 @@ module.exports = { reportMetrics, reportAttack, reportWafUpdate: incrementWafUpdatesMetric, + reportWafConfigError: incrementWafConfigErrorsMetric, reportRaspRuleSkipped: updateRaspRuleSkippedMetricTags, reportDerivatives, finishRequest, diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index c8594770831..83f745e9dec 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -27,14 +27,18 @@ function loadRules (config) { function updateWafFromRC ({ toUnapply, toApply, toModify }) { const newActions = new SpyMap(appliedActions) + let wafUpdated = false + for (const item of toUnapply) { if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue try { waf.wafManager.remove(item.path) + wafUpdated = true } catch (e) { item.apply_state = ERROR item.apply_error = e.toString() + Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } } @@ -46,9 +50,11 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { item.apply_state = updateResult.success ? ACKNOWLEDGED : ERROR if (updateResult.success) { + wafUpdated = true Reporter.reportSuccessfulWafUpdate(item.product, item.id, updateResult.diagnostics) } else { item.apply_error = JSON.stringify(updateResult.diagnostics, DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR) + Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } // check asm actions @@ -61,9 +67,14 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { } catch (e) { item.apply_state = ERROR item.apply_error = e.toString() + Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } } + if (wafUpdated) { + Reporter.reportWafUpdate(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) + } + // Manage blocking actions if (newActions.modified) { appliedActions = newActions diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index 32535b79a27..ccf28666e26 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -13,6 +13,7 @@ const { trackWafMetrics, incrementWafInit, incrementWafUpdates, + incrementWafConfigErrors, incrementWafRequests } = require('./waf') @@ -115,10 +116,16 @@ function incrementWafInitMetric (wafVersion, rulesVersion, success) { incrementWafInit(wafVersion, rulesVersion, success) } -function incrementWafUpdatesMetric (wafVersion, rulesVersion, success) { +function incrementWafUpdatesMetric (wafVersion, rulesVersion) { if (!enabled) return - incrementWafUpdates(wafVersion, rulesVersion, success) + incrementWafUpdates(wafVersion, rulesVersion) +} + +function incrementWafConfigErrorsMetric (wafVersion, rulesVersion) { + if (!enabled) return + + incrementWafConfigErrors(wafVersion, rulesVersion) } function incrementWafRequestsMetric (req) { @@ -167,6 +174,7 @@ module.exports = { updateRaspRuleSkippedMetricTags, incrementWafInitMetric, incrementWafUpdatesMetric, + incrementWafConfigErrorsMetric, incrementWafRequestsMetric, incrementMissingUserLoginMetric, incrementMissingUserIdMetric, diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index bf8d0e07eae..5e2f2e694b5 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -100,13 +100,14 @@ function incrementWafInit (wafVersion, rulesVersion, success) { } } -function incrementWafUpdates (wafVersion, rulesVersion, success) { +function incrementWafUpdates (wafVersion, rulesVersion) { const versionsTags = getVersionsTags(wafVersion, rulesVersion) appsecMetrics.count('waf.updates', { ...versionsTags, success }).inc() +} - if (!success) { - appsecMetrics.count('waf.config_errors', versionsTags).inc() - } +function incrementWafConfigErrors (wafVersion, rulesVersion) { + const versionsTags = getVersionsTags(wafVersion, rulesVersion) + appsecMetrics.count('waf.config_errors', versionsTags).inc() } function incrementWafRequests (store) { @@ -137,5 +138,6 @@ module.exports = { trackWafMetrics, incrementWafInit, incrementWafUpdates, + incrementWafConfigErrors, incrementWafRequests } diff --git a/packages/dd-trace/src/appsec/waf/waf_manager.js b/packages/dd-trace/src/appsec/waf/waf_manager.js index 38bb5c0d0b0..a70f10374a9 100644 --- a/packages/dd-trace/src/appsec/waf/waf_manager.js +++ b/packages/dd-trace/src/appsec/waf/waf_manager.js @@ -55,51 +55,33 @@ class WAFManager { } update (product, rules, path) { - try { - if (product === 'ASM_DD' && this.ddwaf.configPaths.includes(DEFAULT_WAF_CONFIG_PATH)) { - this.ddwaf.removeConfig(DEFAULT_WAF_CONFIG_PATH) - } - - const success = this.ddwaf.createOrUpdateConfig(rules, path) - const diagnostics = this.ddwaf.diagnostics - - if (diagnostics.ruleset_version) { - this.rulesVersion = diagnostics.ruleset_version - } - - Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, success) + if (product === 'ASM_DD' && this.ddwaf.configPaths.includes(DEFAULT_WAF_CONFIG_PATH)) { + this.ddwaf.removeConfig(DEFAULT_WAF_CONFIG_PATH) + } - if (product === 'ASM_DD' && !success && !this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { - this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) - } + const success = this.ddwaf.createOrUpdateConfig(rules, path) + const diagnostics = this.ddwaf.diagnostics - return { success, diagnostics } - } catch (error) { - Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) + if (diagnostics.ruleset_version) { + this.rulesVersion = diagnostics.ruleset_version + } - throw error + if (product === 'ASM_DD' && !success && !this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { + this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) } + return { success, diagnostics } } remove (path) { - try { - this.ddwaf.removeConfig(path) - - if (!this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { - console.log('Reverting default rules') - this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) - } + this.ddwaf.removeConfig(path) - if (this.ddwaf.diagnostics.ruleset_version) { - this.rulesVersion = this.ddwaf.diagnostics.ruleset_version - } - - Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, true) - } catch (error) { - Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) + if (!this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { + this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) + } - throw error + if (this.ddwaf.diagnostics.ruleset_version) { + this.rulesVersion = this.ddwaf.diagnostics.ruleset_version } } From 85e40fdfdcff1aa0ef9f72d944104497dd3afa6f Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 2 May 2025 15:25:02 +0200 Subject: [PATCH 05/22] Remove deprecated span tags --- packages/dd-trace/src/appsec/reporter.js | 8 +------- packages/dd-trace/src/appsec/waf/waf_manager.js | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 84fa068b138..42c14f649d5 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -228,15 +228,9 @@ function getCollectedHeaders (req, res, shouldCollectEventHeaders) { return headersTags } -function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}, success = false) { +function reportWafInit (wafVersion, rulesVersion, success = false) { if (success) { metricsQueue.set('_dd.appsec.waf.version', wafVersion) - - metricsQueue.set('_dd.appsec.event_rules.loaded', diagnosticsRules.loaded?.length || 0) - metricsQueue.set('_dd.appsec.event_rules.error_count', diagnosticsRules.failed?.length || 0) - if (diagnosticsRules.failed?.length) { - metricsQueue.set('_dd.appsec.event_rules.errors', JSON.stringify(diagnosticsRules.errors)) - } } incrementWafInitMetric(wafVersion, rulesVersion, success) diff --git a/packages/dd-trace/src/appsec/waf/waf_manager.js b/packages/dd-trace/src/appsec/waf/waf_manager.js index a70f10374a9..6a0ff1bec4e 100644 --- a/packages/dd-trace/src/appsec/waf/waf_manager.js +++ b/packages/dd-trace/src/appsec/waf/waf_manager.js @@ -16,7 +16,7 @@ class WAFManager { this.rulesVersion = this.ddwaf.diagnostics.ruleset_version this.defaultRules = rules - Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules, true) + Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, true) } _loadDDWAF (rules) { From fb8f7dd574fd806e97da68d26cb85ce2c83fc597 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 2 May 2025 15:42:31 +0200 Subject: [PATCH 06/22] Fix report waf update --- packages/dd-trace/src/appsec/rule_manager.js | 6 +++++- packages/dd-trace/src/appsec/telemetry/index.js | 4 ++-- packages/dd-trace/src/appsec/telemetry/waf.js | 2 +- packages/dd-trace/src/appsec/waf/index.js | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 83f745e9dec..3a90792a517 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -28,6 +28,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { const newActions = new SpyMap(appliedActions) let wafUpdated = false + let wafUpdatedSuccess = true for (const item of toUnapply) { if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue @@ -36,6 +37,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { waf.wafManager.remove(item.path) wafUpdated = true } catch (e) { + wafUpdatedSuccess = false item.apply_state = ERROR item.apply_error = e.toString() Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) @@ -53,6 +55,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { wafUpdated = true Reporter.reportSuccessfulWafUpdate(item.product, item.id, updateResult.diagnostics) } else { + wafUpdatedSuccess = false item.apply_error = JSON.stringify(updateResult.diagnostics, DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR) Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } @@ -65,6 +68,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { } } catch (e) { + wafUpdatedSuccess = false item.apply_state = ERROR item.apply_error = e.toString() Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) @@ -72,7 +76,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { } if (wafUpdated) { - Reporter.reportWafUpdate(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) + Reporter.reportWafUpdate(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion, wafUpdatedSuccess) } // Manage blocking actions diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index ccf28666e26..4b09689d854 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -116,10 +116,10 @@ function incrementWafInitMetric (wafVersion, rulesVersion, success) { incrementWafInit(wafVersion, rulesVersion, success) } -function incrementWafUpdatesMetric (wafVersion, rulesVersion) { +function incrementWafUpdatesMetric (wafVersion, rulesVersion, success) { if (!enabled) return - incrementWafUpdates(wafVersion, rulesVersion) + incrementWafUpdates(wafVersion, rulesVersion, success) } function incrementWafConfigErrorsMetric (wafVersion, rulesVersion) { diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index 5e2f2e694b5..fa720d2d487 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -100,7 +100,7 @@ function incrementWafInit (wafVersion, rulesVersion, success) { } } -function incrementWafUpdates (wafVersion, rulesVersion) { +function incrementWafUpdates (wafVersion, rulesVersion, success) { const versionsTags = getVersionsTags(wafVersion, rulesVersion) appsecMetrics.count('waf.updates', { ...versionsTags, success }).inc() } diff --git a/packages/dd-trace/src/appsec/waf/index.js b/packages/dd-trace/src/appsec/waf/index.js index 94095acb148..bf92a903e8c 100644 --- a/packages/dd-trace/src/appsec/waf/index.js +++ b/packages/dd-trace/src/appsec/waf/index.js @@ -7,7 +7,6 @@ const waf = { wafManager: null, init, destroy, - update, run: noop, disposeContext: noop } @@ -34,8 +33,8 @@ function destroy () { waf.disposeContext = noop } +/* function update (newRules) { - console.log('call update with'/*, newRules*/) // TODO: check race conditions between Appsec enable/disable and WAF updates, the whole RC state management in general if (!waf.wafManager) throw new Error('Cannot update disabled WAF') @@ -46,6 +45,7 @@ function update (newRules) { throw err } } +*/ function run (data, req, raspRule) { if (!req) { From 66712f6ce04d2c5cf1b01aaba87a4f70cab8727c Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Mon, 5 May 2025 15:20:42 +0200 Subject: [PATCH 07/22] Tests --- packages/dd-trace/test/appsec/index.spec.js | 3 ++ .../dd-trace/test/appsec/reporter.spec.js | 18 +++--------- .../test/appsec/telemetry/waf.spec.js | 29 +++++++++++++------ 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js index a96aeeb6ca8..cfb7fcca733 100644 --- a/packages/dd-trace/test/appsec/index.spec.js +++ b/packages/dd-trace/test/appsec/index.spec.js @@ -1266,6 +1266,7 @@ describe('IP blocking', function () { const toModify = [{ product: 'ASM_DATA', id: '1', + path: 'datadog/00/ASM_DATA/test/IP blocking', file: ruleData }] const htmlDefaultContent = blockedTemplate.html @@ -1388,6 +1389,7 @@ describe('IP blocking', function () { const toModifyCustomActions = [{ product: 'ASM', id: 'custom-actions', + path: 'datadog/00/ASM/test/Custom actions/Default content with custom status', file: { actions: [ { @@ -1448,6 +1450,7 @@ describe('IP blocking', function () { const toModifyCustomActions = [{ product: 'ASM', id: 'custom-actions', + path: 'datadog/00/ASM/test/Custom actions/Redirect on error', file: { actions: [ { diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index 76cd1949d98..c5d4d0c6211 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -138,26 +138,19 @@ describe('reporter', () => { } it('should add some entries to metricsQueue', () => { - Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true) + Reporter.reportWafInit(wafVersion, rulesVersion, true) expect(Reporter.metricsQueue.get('_dd.appsec.waf.version')).to.be.eq(wafVersion) - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.eq(3) - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.eq(1) - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.errors')) - .to.be.eq(JSON.stringify(diagnosticsRules.errors)) }) it('should not add entries to metricsQueue with success false', () => { - Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, false) + Reporter.reportWafInit(wafVersion, rulesVersion, false) expect(Reporter.metricsQueue.get('_dd.appsec.waf.version')).to.be.undefined - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.undefined - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.undefined - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.errors')).to.be.undefined }) it('should call incrementWafInitMetric', () => { - Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true) + Reporter.reportWafInit(wafVersion, rulesVersion, true) expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true) }) @@ -167,10 +160,7 @@ describe('reporter', () => { const rulesVersion = undefined const diagnosticsRules = undefined - Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true) - - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.eq(0) - expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.eq(0) + Reporter.reportWafInit(wafVersion, rulesVersion, true) expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true) }) diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index a7b7e3c1eb5..6fea048cfb1 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -267,22 +267,33 @@ describe('Appsec Waf Telemetry metrics', () => { expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2') expect(metrics.series[0].tags).to.include('success:true') }) + }) + + describe('incrementWafConfigErrors', () => { + it('should increment waf.config_errors metric', () => { + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion) - it('should increment waf.updates and waf.config_errors on failed update', () => { + expect(count).to.have.been.calledOnceWithExactly('waf.config_errors', { + waf_version: wafVersion, + event_rules_version: rulesVersion, + }) + expect(inc).to.have.been.calledOnce + }) + + it('should increment waf.config_errors metric multiple times', () => { sinon.restore() - appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion, false) + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion, true) + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion, true) + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion, true) const { metrics } = appsecNamespace.toJSON() - expect(metrics.series.length).to.be.eq(2) - expect(metrics.series[0].metric).to.be.eq('waf.updates') + expect(metrics.series.length).to.be.eq(1) + expect(metrics.series[0].metric).to.be.eq('waf.config_errors') + expect(metrics.series[0].points.length).to.be.eq(1) + expect(metrics.series[0].points[0][1]).to.be.eq(3) expect(metrics.series[0].tags).to.include('waf_version:0.0.1') expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2') - expect(metrics.series[0].tags).to.include('success:false') - - expect(metrics.series[1].metric).to.be.eq('waf.config_errors') - expect(metrics.series[1].tags).to.include('waf_version:0.0.1') - expect(metrics.series[1].tags).to.include('event_rules_version:0.0.2') }) }) From 0417683c4d3b308fde43167156f6bd22e62a15ad Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Mon, 5 May 2025 17:28:15 +0200 Subject: [PATCH 08/22] Tests for Rule Manager --- packages/dd-trace/src/appsec/rule_manager.js | 32 +- .../dd-trace/test/appsec/rule_manager.spec.js | 701 +++++++----------- 2 files changed, 274 insertions(+), 459 deletions(-) diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 3a90792a517..5abbe152606 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -7,11 +7,6 @@ const Reporter = require('./reporter') const blocking = require('./blocking') -const DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR = [ - "error", "errors", - "exclusions", "rules", "processors", "rules_override", "rules_data", "custom_rules", "actions", "scanners" -] - let appliedActions = new Map() function loadRules (config) { @@ -35,7 +30,12 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { try { waf.wafManager.remove(item.path) + item.apply_state = ACKNOWLEDGED wafUpdated = true + + if (item.product === 'ASM') { + newActions.delete(item.id) + } } catch (e) { wafUpdatedSuccess = false item.apply_state = ERROR @@ -56,7 +56,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { Reporter.reportSuccessfulWafUpdate(item.product, item.id, updateResult.diagnostics) } else { wafUpdatedSuccess = false - item.apply_error = JSON.stringify(updateResult.diagnostics, DIAGNOSTICS_KEYS_TO_KEEP_APPLY_ERROR) + item.apply_error = JSON.stringify(extractErrors(updateResult.diagnostics)) Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } @@ -66,7 +66,6 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { newActions.set(item.id, item.file.actions) } } - } catch (e) { wafUpdatedSuccess = false item.apply_state = ERROR @@ -114,6 +113,25 @@ function concatArrays (files) { return [...files.values()].flat() } +function extractErrors (obj) { + if (typeof obj !== 'object' || obj === null) return null + + const result = {} + + for (const [key, value] of Object.entries(obj)) { + if (key === 'error' || key === 'errors') { + result[key] = value + } else if (typeof value === 'object' && value !== null) { + const child = extractErrors(value) + if (child && Object.keys(child).length > 0) { + result[key] = child + } + } + } + + return Object.keys(result).length > 0 ? result : null +} + function clearAllRules () { waf.destroy() appliedActions.clear() diff --git a/packages/dd-trace/test/appsec/rule_manager.spec.js b/packages/dd-trace/test/appsec/rule_manager.spec.js index 0da5e7dbec9..22e9a4802b5 100644 --- a/packages/dd-trace/test/appsec/rule_manager.spec.js +++ b/packages/dd-trace/test/appsec/rule_manager.spec.js @@ -1,13 +1,15 @@ 'use strict' -const path = require('path') +const { assert } = require('chai') const fs = require('fs') -const { loadRules, clearAllRules, updateWafFromRC } = require('../../src/appsec/rule_manager') +const path = require('path') +const { loadRules, clearAllRules } = require('../../src/appsec/rule_manager') const Config = require('../../src/config') -const { ACKNOWLEDGED } = require('../../src/remote_config/apply_states') +const { ACKNOWLEDGED, UNACKNOWLEDGED, ERROR } = require('../../src/remote_config/apply_states') const rules = require('../../src/appsec/recommended.json') const waf = require('../../src/appsec/waf') +const WAFManager = require('../../src/appsec/waf/waf_manager') const blocking = require('../../src/appsec/blocking') describe('AppSec Rule Manager', () => { @@ -19,7 +21,6 @@ describe('AppSec Rule Manager', () => { sinon.stub(waf, 'init') sinon.stub(waf, 'destroy') - sinon.stub(waf, 'update') sinon.stub(blocking, 'setDefaultBlockingActionParameters') }) @@ -72,437 +73,265 @@ describe('AppSec Rule Manager', () => { }) describe('updateWafFromRC', () => { - describe('ASM_DATA', () => { - it('should call update with modified rules', () => { - const rulesData = { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - - const toModify = [{ + function getRcConfigs () { + return { + toUnapply: [{ + id: 'test.toUnapply', + product: 'ASM_DD', + path: 'test/rule_manager/updateWafFromRC/ASM_DD/01', + file: {}, + apply_state: UNACKNOWLEDGED + }], + toModify: [{ + id: 'test.toModify', product: 'ASM_DATA', - id: '1', - file: rulesData + path: 'test/rule_manager/updateWafFromRC/ASM_DATA/01', + file: { + rules_data: [{ + data: [ + { value: '1.2.3.4' } + ], + id: 'blocked_ips', + type: 'data_with_expiration' + }] + }, + apply_state: UNACKNOWLEDGED + }], + toApply: [{ + id: 'test.toApply', + product: 'ASM', + path: 'test/rule_manager/updateWafFromRC/ASM/01', + file: { + exclusions: [{ + ekey: 'eValue' + }], + rules_override: [{ + roKey: 'roValue' + }], + custom_rules: [{ + piKey: 'piValue' + }] + }, + apply_state: UNACKNOWLEDGED }] - - updateWafFromRC({ toUnapply: [], toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnceWithExactly(rulesData) + } + } + + let RuleManager + let reportWafConfigError, reportSuccessfulWafUpdate, reportWafUpdate + let setDefaultBlockingActionParameters + + beforeEach(() => { + reportWafUpdate = sinon.stub() + reportSuccessfulWafUpdate = sinon.stub() + reportWafConfigError = sinon.stub() + setDefaultBlockingActionParameters = sinon.stub() + + RuleManager = proxyquire.noCallThru()('../src/appsec/rule_manager', { + './reporter': { + reportWafUpdate, + reportSuccessfulWafUpdate, + reportWafConfigError + }, + './blocking': { + setDefaultBlockingActionParameters + } }) - it('should apply/modify the last rule with same id', () => { - const rulesDataFirst = { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } + waf.init.callThrough() - const rulesDataSecond = { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } + WAFManager.prototype.update = sinon.stub() + WAFManager.prototype.remove = sinon.stub() - const toModify = [ - { - product: 'ASM_DATA', - id: '1', - file: rulesDataFirst - }, - { - product: 'ASM_DATA', - id: '1', - file: rulesDataSecond - } - ] + RuleManager.clearAllRules() + config = new Config() + RuleManager.loadRules(config.appsec) + sinon.resetHistory() + }) - const expectedPayload = { - rules_data: [ - { data: [{ value: '4.3.2.1' }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + it('should not apply configs from non ASM products', () => { + const rcConfigsForNonAsmProducts = { + toUnapply: [{ + id: 'test.toUnapply', + product: 'NON_ASM_PRODUCT', + path: 'test/rule_manager/updateWafFromRC/NON_ASM_PRODUCT/01', + file: {}, + apply_state: UNACKNOWLEDGED + }], + toModify: [{ + id: 'test.toModify', + product: 'NON_ASM_PRODUCT', + path: 'test/rule_manager/updateWafFromRC/NON_ASM_PRODUCT/02', + file: {}, + apply_state: UNACKNOWLEDGED + }], + toApply: [{ + id: 'test.toApply', + product: 'NON_ASM_PRODUCT', + path: 'test/rule_manager/updateWafFromRC/NON_ASM_PRODUCT/03', + file: {}, + apply_state: UNACKNOWLEDGED + }] + } - updateWafFromRC({ toUnapply: [], toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) + assert.doesNotThrow(() => { + RuleManager.updateWafFromRC(rcConfigsForNonAsmProducts) }) - it('should merge all apply/modify rules', () => { - const toModify = [ - { - product: 'ASM_DATA', - id: '1', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - }, - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + sinon.assert.notCalled(waf.wafManager.update) + sinon.assert.notCalled(waf.wafManager.remove) + assert.strictEqual(rcConfigsForNonAsmProducts.toUnapply[0].apply_state, UNACKNOWLEDGED) + assert.notProperty(rcConfigsForNonAsmProducts.toUnapply[0], 'apply_error') + assert.strictEqual(rcConfigsForNonAsmProducts.toModify[0].apply_state, UNACKNOWLEDGED) + assert.notProperty(rcConfigsForNonAsmProducts.toModify[0], 'apply_error') + assert.strictEqual(rcConfigsForNonAsmProducts.toApply[0].apply_state, UNACKNOWLEDGED) + assert.notProperty(rcConfigsForNonAsmProducts.toApply[0], 'apply_error') + }) - const expectedPayload = { - rules_data: [ - { data: [{ value: '1.2.3.4' }, { value: '4.3.2.1' }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + it('should apply configs from ASM products', () => { + const rcConfigs = getRcConfigs() + + RuleManager.updateWafFromRC(rcConfigs) + + sinon.assert.calledOnceWithExactly(waf.wafManager.remove, rcConfigs.toUnapply[0].path) + sinon.assert.calledTwice(waf.wafManager.update) + sinon.assert.calledWith( + waf.wafManager.update.getCall(0), + rcConfigs.toApply[0].product, + rcConfigs.toApply[0].file, + rcConfigs.toApply[0].path + ) + sinon.assert.calledWith( + waf.wafManager.update.getCall(1), + rcConfigs.toModify[0].product, + rcConfigs.toModify[0].file, + rcConfigs.toModify[0].path + ) + }) - updateWafFromRC({ toUnapply: [], toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) - }) + it('should update apply_state and apply_error on successful apply', () => { + WAFManager.prototype.update.returns({ success: true }) - it('should merge all apply/modify and unapply rules', () => { - const toModify = [ - { - product: 'ASM_DATA', - id: '1', - file: { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - }, - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + const rcConfigs = getRcConfigs() - const toUnapply = [ - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + RuleManager.updateWafFromRC(rcConfigs) - const expectedPayload = { - rules_data: [ - { data: [{ value: '4.3.2.1' }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + assert.strictEqual(rcConfigs.toUnapply[0].apply_state, ACKNOWLEDGED) + assert.notProperty(rcConfigs.toUnapply[0], 'apply_error') + assert.strictEqual(rcConfigs.toModify[0].apply_state, ACKNOWLEDGED) + assert.notProperty(rcConfigs.toModify[0], 'apply_error') + assert.strictEqual(rcConfigs.toApply[0].apply_state, ACKNOWLEDGED) + assert.notProperty(rcConfigs.toApply[0], 'apply_error') + }) - updateWafFromRC({ toUnapply, toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) - }) + it('should update apply_state and apply_error on failed config remove', () => { + const removeConfigError = new Error('Error remove config') + WAFManager.prototype.remove.throws(removeConfigError) - it('should merge all apply/modify rules with different expiration', () => { - // TODO: use case from previous tests, not sure if this can happen. - const toApply = [ - { - product: 'ASM_DATA', - id: '1', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4', expiration: 200 } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - }, - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4', expiration: 100 } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + const { toUnapply } = getRcConfigs() - const expectedPayload = { - rules_data: [ - { data: [{ value: '1.2.3.4', expiration: 200 }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + RuleManager.updateWafFromRC({ toUnapply, toApply: [], toModify: [] }) - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) - }) + assert.strictEqual(toUnapply[0].apply_state, ERROR) + assert.strictEqual(toUnapply[0].apply_error, removeConfigError.toString()) }) - describe('ASM_DD', () => { - beforeEach(() => { - loadRules(config.appsec) - }) - - it('should apply new rules', () => { - const testRules = { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [] - }], - processors: [{ - id: 'test-processor-id', - generator: 'test-generator', - evaluate: false, - output: true - }], - scanners: [{ - id: 'test-scanner-id', - name: 'Test name', - key: { - operator: 'match_regex', - parameters: { - regex: 'test-regex' - } - }, - value: { - operator: 'match_regex', - parameters: { - regex: 'test-regex-2' - } - }, - tags: { - type: 'card', - card_type: 'test', - category: 'payment' - } - }] - } - - const toApply = [ - { - product: 'ASM_DD', - id: '1', - file: testRules + it('should update apply_state and apply_error on failed config update', () => { + const diagnostics = { + rules: { + loaded: [], + failed: ['blk-001-001'], + skipped: [], + errors: { + 'missing key operator': [ + 'blk-001-001' + ] + }, + warnings: { + 'invalid tag': [ + 'blk-001-001' + ] + } + }, + processors: { + loaded: ['http-endpoint-fingerprint'], + failed: [], + skipped: [], + warnings: { + 'no mappings defined': [ + 'http-endpoint-fingerprint' + ] } - ] - - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledOnceWithExactly(testRules) - }) - - it('should maintain previously added exclusions and rules_overrides', () => { - const asm = { - exclusions: [{ - ekey: 'eValue' - }] - } - const testRules = { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [] - }], - processors: [{ - id: 'test-processor-id', - generator: 'test-generator', - evaluate: false, - output: true - }], - scanners: [{ - id: 'test-scanner-id', - name: 'Test name', - key: { - operator: 'match_regex', - parameters: { - regex: 'test-regex' - } - }, - value: { - operator: 'match_regex', - parameters: { - regex: 'test-regex-2' - } - }, - tags: { - type: 'card', - card_type: 'test', - category: 'payment' - } - }] } + } + WAFManager.prototype.update.returns({ success: false, diagnostics }) - const toApply = [ - { - product: 'ASM', - id: '1', - file: asm - }, - { - product: 'ASM_DD', - id: '2', - file: testRules - } - ] + const { toModify, toApply } = getRcConfigs() - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledWithExactly({ ...testRules, ...asm }) - }) + RuleManager.updateWafFromRC({ toUnapply: [], toApply, toModify }) - it('should support hotswapping ruleset in same batch', () => { - const rules1 = { - product: 'ASM_DD', - id: 'rules1', - file: { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [ - { - parameters: { - inputs: [ - { address: 'http.test' } - ], - data: 'blocked_ips' - }, - operator: 'ip_match' - } - ] - }] - } - } + assert.strictEqual(toApply[0].apply_state, ERROR) + assert.strictEqual(toApply[0].apply_error, JSON.stringify({ rules: { errors: diagnostics.rules.errors } })) + assert.strictEqual(toModify[0].apply_state, ERROR) + assert.strictEqual(toModify[0].apply_error, JSON.stringify({ rules: { errors: diagnostics.rules.errors } })) + }) - const rules2 = { - product: 'ASM_DD', - id: 'rules2', - file: { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [ - { - parameters: { - inputs: [ - { address: 'http.test' } - ], - data: 'blocked_ips' - }, - operator: 'ip_match' - } - ] - }] - } - } + it('should report successful waf update', () => { + WAFManager.prototype.update.returns({ success: true, diagnostics: {} }) - updateWafFromRC({ toUnapply: [], toApply: [rules1], toModify: [] }) + const rcConfigs = getRcConfigs() - updateWafFromRC({ toUnapply: [rules1], toApply: [rules2], toModify: [] }) + RuleManager.updateWafFromRC(rcConfigs) - expect(rules1.apply_state).to.equal(ACKNOWLEDGED) - expect(rules1.apply_error).to.equal(undefined) - expect(rules2.apply_state).to.equal(ACKNOWLEDGED) - expect(rules2.apply_error).to.equal(undefined) + sinon.assert.calledOnceWithExactly( + reportWafUpdate, + waf.wafManager.ddwafVersion, + waf.wafManager.rulesVersion, + true + ) + sinon.assert.calledTwice(reportSuccessfulWafUpdate) + sinon.assert.calledWith(reportSuccessfulWafUpdate, rcConfigs.toModify[0].product, rcConfigs.toModify[0].id, {}) + sinon.assert.calledWith(reportSuccessfulWafUpdate, rcConfigs.toApply[0].product, rcConfigs.toApply[0].id, {}) + }) + + it('should report waf config error', () => { + WAFManager.prototype.remove.throws(new Error('Error removing config')) + WAFManager.prototype.update.returns({ success: false, diagnostics: {} }) + + const rcConfigs = getRcConfigs() + + RuleManager.updateWafFromRC(rcConfigs) + sinon.assert.notCalled(reportWafUpdate) + sinon.assert.calledThrice(reportWafConfigError) + reportWafConfigError.getCalls().forEach((spyCall) => { + sinon.assert.calledWithExactly( + spyCall, + waf.wafManager.ddwafVersion, + waf.wafManager.rulesVersion + ) }) }) - describe('ASM', () => { - it('should apply both rules_override and exclusions', () => { - const asm = { - exclusions: [{ - ekey: 'eValue' - }], - rules_override: [{ - roKey: 'roValue' - }], - custom_rules: [{ - piKey: 'piValue' - }] - } + it('should report waf update', () => { + WAFManager.prototype.update.onFirstCall().returns({ success: false, diagnostics: {} }) + WAFManager.prototype.update.onSecondCall().returns({ success: true, diagnostics: {} }) - const toApply = [ - { - product: 'ASM', - id: '1', - file: asm - } - ] + const rcConfigs = getRcConfigs() - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + RuleManager.updateWafFromRC(rcConfigs) - expect(waf.update).to.have.been.calledOnceWithExactly(asm) - }) + sinon.assert.calledOnceWithExactly( + reportWafUpdate, + waf.wafManager.ddwafVersion, + waf.wafManager.rulesVersion, + false + ) + }) + describe('ASM', () => { it('should apply blocking actions', () => { + WAFManager.prototype.update.returns({ success: true }) + const toApply = [ { product: 'ASM', @@ -536,32 +365,31 @@ describe('AppSec Rule Manager', () => { } ] - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + RuleManager.updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - const expectedPayload = { - actions: [ - { - id: 'notblock', - parameters: { - location: '/notfound', - status_code: 404 - } - }, - { - id: 'block', - parameters: { - location: '/redirected', - status_code: 302 - } + const expectedActions = [ + { + id: 'notblock', + parameters: { + location: '/notfound', + status_code: 404 } - ] - } + }, + { + id: 'block', + parameters: { + location: '/redirected', + status_code: 302 + } + } + ] - expect(waf.update).to.have.been.calledOnceWithExactly(expectedPayload) - expect(blocking.setDefaultBlockingActionParameters).to.have.been.calledOnceWithExactly(expectedPayload.actions) + sinon.assert.calledOnceWithExactly(setDefaultBlockingActionParameters, expectedActions) }) it('should unapply blocking actions', () => { + WAFManager.prototype.update.returns({ success: true }) + const asm = { actions: [ { @@ -581,11 +409,10 @@ describe('AppSec Rule Manager', () => { file: asm } ] - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledOnceWithExactly(asm) - expect(blocking.setDefaultBlockingActionParameters).to.have.been.calledOnceWithExactly(asm.actions) + RuleManager.updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + sinon.assert.calledOnceWithExactly(setDefaultBlockingActionParameters, asm.actions) sinon.resetHistory() const toUnapply = [ @@ -595,39 +422,9 @@ describe('AppSec Rule Manager', () => { } ] - updateWafFromRC({ toUnapply, toApply: [], toModify: [] }) - - expect(waf.update).to.have.been.calledOnceWithExactly({ actions: [] }) - expect(blocking.setDefaultBlockingActionParameters).to.have.been.calledOnceWithExactly([]) - }) - - it('should ignore other properties', () => { - const asm = { - exclusions: [{ - ekey: 'eValue' - }], - rules_override: [{ - roKey: 'roValue' - }], - not_supported: [{ - nsKey: 'nsValue' - }] - } - - const toApply = [ - { - product: 'ASM', - id: '1', - file: asm - } - ] - - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + RuleManager.updateWafFromRC({ toUnapply, toApply: [], toModify: [] }) - expect(waf.update).to.have.been.calledOnceWithExactly({ - exclusions: asm.exclusions, - rules_override: asm.rules_override - }) + sinon.assert.calledOnceWithExactly(setDefaultBlockingActionParameters, []) }) }) }) From ec54f44d5872ced25dde384a7b396accb1d3773e Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Tue, 6 May 2025 12:10:05 +0200 Subject: [PATCH 09/22] More tests --- packages/dd-trace/src/appsec/reporter.js | 20 ++++- .../dd-trace/test/appsec/reporter.spec.js | 80 +++++++++++++++++-- 2 files changed, 89 insertions(+), 11 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 42c14f649d5..77d3d116337 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -244,7 +244,11 @@ function reportSuccessfulWafUpdate (product, rcConfigId, diagnostics) { telemetryLogCh.publish({ message: diagnostics[configKey].error, level: 'ERROR', - tags: `log_type:rc::${product}::diagnostic, appsec_config_key:${configKey}, rc_config_id:${rcConfigId}` + tags: { + log_type: `rc::${product.toLowerCase()}::diagnostic`, + appsec_config_key: configKey, + rc_config_id: rcConfigId + } }) continue } @@ -254,7 +258,11 @@ function reportSuccessfulWafUpdate (product, rcConfigId, diagnostics) { telemetryLogCh.publish({ message: `"${errorMessage}": ${JSON.stringify(errorIds)}`, level: 'ERROR', - tags: `log_type:rc::${product}::diagnostic, appsec_config_key:${configKey}, rc_config_id:${rcConfigId}` + tags: { + log_type: `rc::${product.toLowerCase()}::diagnostic`, + appsec_config_key: configKey, + rc_config_id: rcConfigId + } }) } } @@ -263,8 +271,12 @@ function reportSuccessfulWafUpdate (product, rcConfigId, diagnostics) { for (const [warningMessage, warningIds] of Object.entries(diagnostics[configKey].warnings)) { telemetryLogCh.publish({ message: `"${warningMessage}": ${JSON.stringify(warningIds)}`, - level: 'ERROR', - tags: `log_type:rc::${product}::diagnostic, appsec_config_key:${configKey}, rc_config_id:${rcConfigId}` + level: 'WARN', + tags: { + log_type: `rc::${product.toLowerCase()}::diagnostic`, + appsec_config_key: configKey, + rc_config_id: rcConfigId + } }) } } diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index c5d4d0c6211..b5c5723d260 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -1,8 +1,10 @@ 'use strict' +const dc = require('dc-polyfill') const proxyquire = require('proxyquire') -const { storage } = require('../../../datadog-core') const zlib = require('zlib') + +const { storage } = require('../../../datadog-core') const { ASM } = require('../../src/standalone/product') const { USER_KEEP } = require('../../../../ext/priority') @@ -131,11 +133,6 @@ describe('reporter', () => { describe('reportWafInit', () => { const wafVersion = '0.0.1' const rulesVersion = '0.0.2' - const diagnosticsRules = { - loaded: ['1', '3', '4'], - failed: ['2'], - errors: { error: 'error parsing rule 2' } - } it('should add some entries to metricsQueue', () => { Reporter.reportWafInit(wafVersion, rulesVersion, true) @@ -158,7 +155,6 @@ describe('reporter', () => { it('should not fail with undefined arguments', () => { const wafVersion = undefined const rulesVersion = undefined - const diagnosticsRules = undefined Reporter.reportWafInit(wafVersion, rulesVersion, true) @@ -289,6 +285,76 @@ describe('reporter', () => { }) }) + describe('reportSuccessfulWafUpdate', () => { + it('should send diagnostics using telemetry logs', () => { + const telemetryLogHandlerAssert = sinon.stub() + + const telemetryLogCh = dc.channel('datadog:telemetry:log') + telemetryLogCh.subscribe(telemetryLogHandlerAssert) + + const product = 'ASM_DD' + const rcConfigId = '1' + const diagnostics = { + rules: { + loaded: [], + failed: ['blk-001-001'], + skipped: [], + errors: { + 'missing key operator': [ + 'blk-001-001' + ] + }, + warnings: { + 'invalid tag': [ + 'blk-001-001' + ] + } + }, + processors: { + loaded: ['http-endpoint-fingerprint'], + failed: [], + skipped: [], + warnings: { + 'no mappings defined': [ + 'http-endpoint-fingerprint' + ] + } + } + } + + Reporter.reportSuccessfulWafUpdate(product, rcConfigId, diagnostics) + + expect(telemetryLogHandlerAssert).to.have.been.calledThrice + expect(telemetryLogHandlerAssert.getCall(0)).to.have.been.calledWithExactly({ + message: '"missing key operator": ["blk-001-001"]', + level: 'ERROR', + tags: { + log_type: 'rc::asm_dd::diagnostic', + appsec_config_key: 'rules', + rc_config_id: '1' + } + }, 'datadog:telemetry:log') + expect(telemetryLogHandlerAssert.getCall(1)).to.have.been.calledWithExactly({ + message: '"invalid tag": ["blk-001-001"]', + level: 'WARN', + tags: { + log_type: 'rc::asm_dd::diagnostic', + appsec_config_key: 'rules', + rc_config_id: '1' + } + }, 'datadog:telemetry:log') + expect(telemetryLogHandlerAssert.getCall(2)).to.have.been.calledWithExactly({ + message: '"no mappings defined": ["http-endpoint-fingerprint"]', + level: 'WARN', + tags: { + log_type: 'rc::asm_dd::diagnostic', + appsec_config_key: 'processors', + rc_config_id: '1' + } + }, 'datadog:telemetry:log') + }) + }) + describe('reportAttack', () => { let req From 5174d9785e80de14c351ee0b4302614a6cbc9b5b Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Tue, 6 May 2025 15:01:45 +0200 Subject: [PATCH 10/22] Improve assert --- packages/dd-trace/test/appsec/rule_manager.spec.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/dd-trace/test/appsec/rule_manager.spec.js b/packages/dd-trace/test/appsec/rule_manager.spec.js index 22e9a4802b5..d4f524a7cc5 100644 --- a/packages/dd-trace/test/appsec/rule_manager.spec.js +++ b/packages/dd-trace/test/appsec/rule_manager.spec.js @@ -303,13 +303,11 @@ describe('AppSec Rule Manager', () => { RuleManager.updateWafFromRC(rcConfigs) sinon.assert.notCalled(reportWafUpdate) sinon.assert.calledThrice(reportWafConfigError) - reportWafConfigError.getCalls().forEach((spyCall) => { - sinon.assert.calledWithExactly( - spyCall, - waf.wafManager.ddwafVersion, - waf.wafManager.rulesVersion - ) - }) + sinon.assert.alwaysCalledWithExactly( + reportWafConfigError, + waf.wafManager.ddwafVersion, + waf.wafManager.rulesVersion + ) }) it('should report waf update', () => { From 00b9cdd8d835fd1c32d2f595f079a512fe751b36 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Tue, 6 May 2025 16:30:52 +0200 Subject: [PATCH 11/22] Tests --- .../test/appsec/waf/waf_manager.spec.js | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/packages/dd-trace/test/appsec/waf/waf_manager.spec.js b/packages/dd-trace/test/appsec/waf/waf_manager.spec.js index 9093f43d921..ee7615e3b59 100644 --- a/packages/dd-trace/test/appsec/waf/waf_manager.spec.js +++ b/packages/dd-trace/test/appsec/waf/waf_manager.spec.js @@ -12,6 +12,9 @@ describe('WAFManager', () => { DDWAF.prototype.knownAddresses = knownAddresses DDWAF.prototype.diagnostics = {} DDWAF.prototype.createContext = sinon.stub() + DDWAF.prototype.createOrUpdateConfig = sinon.stub() + DDWAF.prototype.removeConfig = sinon.stub() + DDWAF.prototype.configPaths = [] WAFContextWrapper = sinon.stub() WAFManager = proxyquire('../../../src/appsec/waf/waf_manager', { @@ -30,4 +33,165 @@ describe('WAFManager', () => { sinon.assert.calledOnceWithMatch(WAFContextWrapper, any, any, any, any, knownAddresses) }) }) + + describe('WAF configurations handling', () => { + let wafManager + + const DEFAULT_RULES = { + version: '2.2', + metadata: { + rules_version: '1.14.2' + }, + rules: [ + { + id: 'blk-001-001', + name: 'Block IP Addresses', + tags: { + type: 'block_ip', + category: 'security_response', + module: 'network-acl' + }, + conditions: [ + { + parameters: { + inputs: [ + { + address: 'http.client_ip' + } + ], + data: 'blocked_ips' + }, + operator: 'ip_match' + } + ], + transformers: [], + on_match: [ + 'block' + ] + } + ] + } + + const ASM_CONFIG = { + rules_override: [], + actions: [], + exclusions: [], + custom_rules: [] + } + + const ASM_DATA_CONFIG = { + rules_data: [ + { + data: [ + { + expiration: 1661848350, + value: '188.243.182.156' + }, + { + expiration: 1661848350, + value: '51.222.158.205' + } + ], + id: 'blocked_ips', + type: 'ip_with_expiration' + } + ] + } + + const ASM_DD_CONFIG = { + version: '2.2', + metadata: { + rules_version: '1.42.11' + }, + rules: [] + } + + beforeEach(() => { + wafManager = new WAFManager(DEFAULT_RULES, {}) + }) + + afterEach(() => { + sinon.restore() + }) + + describe('update config', () => { + it('should update WAF config - ASM / ASM_DATA', () => { + DDWAF.prototype.configPaths = ['datadog/00/ASM_DD/default/config'] + + wafManager.update( + 'ASM', + ASM_CONFIG, + 'datadog/00/ASM/test/update_config' + ) + + wafManager.update( + 'ASM_DATA', + ASM_DATA_CONFIG, + 'datadog/00/ASM_DATA/test/update_config' + ) + + sinon.assert.calledWithExactly( + DDWAF.prototype.createOrUpdateConfig.getCall(0), + ASM_CONFIG, + 'datadog/00/ASM/test/update_config' + ) + sinon.assert.calledWithExactly( + DDWAF.prototype.createOrUpdateConfig.getCall(1), + ASM_DATA_CONFIG, + 'datadog/00/ASM_DATA/test/update_config' + ) + }) + + it('should remove default rules on ASM_DD update', () => { + DDWAF.prototype.configPaths = ['datadog/00/ASM_DD/default/config'] + + wafManager.update('ASM_DD', ASM_DD_CONFIG, 'datadog/00/ASM_DD/test/update_config') + + sinon.assert.calledOnceWithExactly( + DDWAF.prototype.removeConfig, + 'datadog/00/ASM_DD/default/config' + ) + sinon.assert.calledOnceWithExactly( + DDWAF.prototype.createOrUpdateConfig, + ASM_DD_CONFIG, + 'datadog/00/ASM_DD/test/update_config' + ) + }) + + it('should apply default rules when no ASM config is present after config update fail', () => { + DDWAF.prototype.configPaths = [] + DDWAF.prototype.createOrUpdateConfig.returns(false) + + wafManager.update('ASM_DD', ASM_DD_CONFIG, 'datadog/00/ASM_DD/test/update_config') + + sinon.assert.calledWithExactly( + DDWAF.prototype.createOrUpdateConfig.getCall(1), + DEFAULT_RULES, + 'datadog/00/ASM_DD/default/config' + ) + }) + }) + + describe('remove config', () => { + it('should remove WAF config', () => { + DDWAF.prototype.configPaths = ['datadog/00/ASM_DD/default/config'] + + wafManager.remove('path/to/remove') + + sinon.assert.calledOnceWithExactly(DDWAF.prototype.removeConfig, 'path/to/remove') + }) + + it('should apply default rules when no ASM config is present after config removal', () => { + DDWAF.prototype.configPaths = [] + + wafManager.remove('path/to/remove') + + sinon.assert.calledOnceWithExactly( + DDWAF.prototype.createOrUpdateConfig, + DEFAULT_RULES, + 'datadog/00/ASM_DD/default/config' + ) + }) + }) + }) }) From efbf2a68ad8c197b43e5b667809e3833bd58ec50 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Tue, 6 May 2025 16:38:59 +0200 Subject: [PATCH 12/22] Clean up --- packages/dd-trace/src/remote_config/index.js | 7 +++---- packages/dd-trace/src/remote_config/manager.js | 4 ---- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/dd-trace/src/remote_config/index.js b/packages/dd-trace/src/remote_config/index.js index 55702586227..ab149bdf000 100644 --- a/packages/dd-trace/src/remote_config/index.js +++ b/packages/dd-trace/src/remote_config/index.js @@ -6,7 +6,6 @@ const RemoteConfigManager = require('./manager') const RemoteConfigCapabilities = require('./capabilities') const { setCollectionMode } = require('../appsec/user_tracking') const log = require('../log') -const RuleManager = require("../appsec/rule_manager"); let rc @@ -136,9 +135,9 @@ function disableWafUpdate () { rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_SHI, false) rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_CMDI, false) - //rc.removeProductHandler('ASM_DATA') - //rc.removeProductHandler('ASM_DD') - //rc.removeProductHandler('ASM') + rc.removeProductHandler('ASM_DATA') + rc.removeProductHandler('ASM_DD') + rc.removeProductHandler('ASM') rc.off(RemoteConfigManager.kPreUpdate, RuleManager.updateWafFromRC) } diff --git a/packages/dd-trace/src/remote_config/manager.js b/packages/dd-trace/src/remote_config/manager.js index dbff5a96c0e..a95f281fb86 100644 --- a/packages/dd-trace/src/remote_config/manager.js +++ b/packages/dd-trace/src/remote_config/manager.js @@ -270,7 +270,6 @@ class RemoteConfigManager extends EventEmitter { for (const item of list) { // TODO: we need a way to tell if unapply configs were handled by kPreUpdate or not, because they're always // emitted unlike the apply and modify configs - console.log('\n\n@@ Dispatching item - before', item.apply_state, item.apply_error) this._callHandlerFor(action, item) @@ -279,7 +278,6 @@ class RemoteConfigManager extends EventEmitter { } else { this.appliedConfigs.set(item.path, item) } - console.log('@@ Dispatching item - after', item.apply_state, item.apply_error) } } @@ -307,9 +305,7 @@ class RemoteConfigManager extends EventEmitter { // If the handler doesn't accept an `ack` callback, assume `apply_state` is `ACKNOWLEDGED`, // unless it returns a promise, in which case we wait for the promise to be resolved or rejected. // TODO: do we want to pass old and new config ? - console.log('@@ _callHandlerFor before handler', item.apply_state, item.apply_error) const result = handler(action, item.file, item.id) - console.log('@@ _callHandlerFor after handler', item.apply_state, item.apply_error) if (result instanceof Promise) { result.then( () => { item.apply_state = ACKNOWLEDGED }, From 50da11a73fe69e93ff48cf934478b430329023d8 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Tue, 6 May 2025 16:56:40 +0200 Subject: [PATCH 13/22] Clean up --- .../dd-trace/test/appsec/waf/index.spec.js | 107 ------------------ 1 file changed, 107 deletions(-) diff --git a/packages/dd-trace/test/appsec/waf/index.spec.js b/packages/dd-trace/test/appsec/waf/index.spec.js index c7ff8682705..8f7b717ca6d 100644 --- a/packages/dd-trace/test/appsec/waf/index.spec.js +++ b/packages/dd-trace/test/appsec/waf/index.spec.js @@ -27,11 +27,6 @@ describe('WAF Manager', () => { DDWAF.version = sinon.stub().returns('1.2.3') DDWAF.prototype.dispose = sinon.stub() DDWAF.prototype.createContext = sinon.stub() - DDWAF.prototype.update = sinon.stub().callsFake(function (newRules) { - if (newRules?.metadata?.rules_version) { - this.diagnostics.ruleset_version = newRules?.metadata?.rules_version - } - }) DDWAF.prototype.diagnostics = { ruleset_version: '1.0.0', rules: { @@ -51,7 +46,6 @@ describe('WAF Manager', () => { sinon.stub(Reporter.metricsQueue, 'set') sinon.stub(Reporter, 'reportMetrics') sinon.stub(Reporter, 'reportAttack') - sinon.stub(Reporter, 'reportWafUpdate') sinon.stub(Reporter, 'reportDerivatives') sinon.spy(Reporter, 'reportWafInit') @@ -74,7 +68,6 @@ describe('WAF Manager', () => { expect(Reporter.reportWafInit).to.have.been.calledWithExactly( '1.2.3', '1.0.0', - { loaded: ['rule_1'], failed: [] }, true ) }) @@ -97,31 +90,6 @@ describe('WAF Manager', () => { waf.init(rules, config.appsec) expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.waf.version', '1.2.3') - expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.loaded', 1) - expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.error_count', 0) - expect(Reporter.metricsQueue.set).not.to.have.been.calledWith('_dd.appsec.event_rules.errors') - }) - - it('should set init metrics with errors', () => { - DDWAF.version.returns('2.3.4') - DDWAF.prototype.diagnostics = { - rules: { - loaded: ['rule_1'], - failed: ['rule_2', 'rule_3'], - errors: { - error_1: ['invalid_1'], - error_2: ['invalid_2', 'invalid_3'] - } - } - } - - waf.init(rules, config.appsec) - - expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.waf.version', '2.3.4') - expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.loaded', 1) - expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.error_count', 2) - expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.errors', - '{"error_1":["invalid_1"],"error_2":["invalid_2","invalid_3"]}') }) }) @@ -170,81 +138,6 @@ describe('WAF Manager', () => { }) }) - describe('wafManager.update', () => { - const wafVersion = '2.3.4' - - beforeEach(() => { - DDWAF.version.returns(wafVersion) - - waf.init(rules, config.appsec) - }) - - it('should call ddwaf.update', () => { - const rules = { - rules_data: [ - { - id: 'blocked_users', - type: 'data_with_expiration', - data: [ - { - expiration: 9999999999, - value: 'user1' - } - ] - } - ] - } - - waf.update(rules) - - expect(DDWAF.prototype.update).to.be.calledOnceWithExactly(rules) - expect(Reporter.reportWafUpdate).to.be.calledOnceWithExactly(wafVersion, '1.0.0', true) - }) - - it('should call Reporter.reportWafUpdate on successful update', () => { - const rules = { - metadata: { - rules_version: '4.2.0' - }, - rules_data: [ - { - id: 'blocked_users', - type: 'data_with_expiration', - data: [ - { - expiration: 9999999999, - value: 'user1' - } - ] - } - ] - } - - waf.update(rules) - - expect(Reporter.reportWafUpdate).to.be.calledOnceWithExactly(wafVersion, '4.2.0', true) - }) - - it('should call Reporter.reportWafUpdate on failed update', () => { - const rules = { - metadata: { - rules_version: '4.2.0' - } - } - const error = new Error('Failed to update rules') - - DDWAF.prototype.update = sinon.stub().throws(error) - - try { - waf.update(rules) - expect.fail('waf.update should have thrown an error') - } catch (err) { - expect(err).to.equal(error) - expect(Reporter.reportWafUpdate).to.be.calledOnceWithExactly(wafVersion, 'unknown', false) - } - }) - }) - describe('WAFContextWrapper', () => { let ddwafContext, wafContextWrapper, req From 1d797707fd6a43c11d779d940b978e55cccca017 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Wed, 28 May 2025 08:03:58 +0200 Subject: [PATCH 14/22] Native appsec package bump --- package.json | 7 ++----- yarn.lock | 8 ++++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index b2954668668..f7bfdcca401 100644 --- a/package.json +++ b/package.json @@ -84,7 +84,7 @@ }, "dependencies": { "@datadog/libdatadog": "^0.5.1", - "@datadog/native-appsec": "file:datadog-native-appsec-8.5.2.tgz", + "@datadog/native-appsec": "9.0.0", "@datadog/native-iast-taint-tracking": "4.0.0", "@datadog/native-metrics": "^3.1.1", "@datadog/pprof": "5.8.0", @@ -153,8 +153,5 @@ "tap": "^16.3.10", "tiktoken": "^1.0.21", "yaml": "^2.8.0" - }, - "bundledDependencies": [ - "@datadog/native-appsec" - ] + } } diff --git a/yarn.lock b/yarn.lock index f7ee0b29250..17bb9e26fa6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -262,10 +262,10 @@ resolved "https://registry.yarnpkg.com/@datadog/libdatadog/-/libdatadog-0.5.1.tgz#fe5c101c457998b74cb66f555f63197b34cad4ba" integrity sha512-KsdOxTUmtjoygaZInSS5U0+KnqoxPKGpcBjGgOHR9NDKfXzmbpy5AmoaPL7JxmMxQzwknpxSi7qzBOSB3yMoJg== -"@datadog/native-appsec@8.5.2": - version "8.5.2" - resolved "https://registry.yarnpkg.com/@datadog/native-appsec/-/native-appsec-8.5.2.tgz#93a2c15c71c2a90e19e12506fbbdec9ccbc91541" - integrity sha512-lETBaVhBk+9o0pc+LDnXvp2ImDyT8K2deuqLf8A6q4/QjzCCXyR/yZO9R5+Kdoc93jZMRTWV9Pr4pBwHEdJSVA== +"@datadog/native-appsec@9.0.0": + version "9.0.0" + resolved "https://registry.yarnpkg.com/@datadog/native-appsec/-/native-appsec-9.0.0.tgz#3ac854d8597ca75af0aa534aae28fa7f13057d2a" + integrity sha512-C7v16pP4p4Y+Cx0jcxTYmhZTptfVs8TYbn6LH/aQgTkwx2tWsWN5lss7fjBYjWyZoPMwVh2UX/yDm4ES25hJnQ== dependencies: node-gyp-build "^3.9.0" From 2ad92a6bd3035f1e2a9dc60fdd9bf9bdbcb38390 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 30 May 2025 02:55:48 +0200 Subject: [PATCH 15/22] Revert "Remove deprecated span tags" This reverts commit f8d8805062447edb1bb13b633ed41fbab477021d. --- packages/dd-trace/src/appsec/reporter.js | 8 +++++++- packages/dd-trace/src/appsec/waf/waf_manager.js | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index 77d3d116337..e53d6d05c24 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -228,9 +228,15 @@ function getCollectedHeaders (req, res, shouldCollectEventHeaders) { return headersTags } -function reportWafInit (wafVersion, rulesVersion, success = false) { +function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}, success = false) { if (success) { metricsQueue.set('_dd.appsec.waf.version', wafVersion) + + metricsQueue.set('_dd.appsec.event_rules.loaded', diagnosticsRules.loaded?.length || 0) + metricsQueue.set('_dd.appsec.event_rules.error_count', diagnosticsRules.failed?.length || 0) + if (diagnosticsRules.failed?.length) { + metricsQueue.set('_dd.appsec.event_rules.errors', JSON.stringify(diagnosticsRules.errors)) + } } incrementWafInitMetric(wafVersion, rulesVersion, success) diff --git a/packages/dd-trace/src/appsec/waf/waf_manager.js b/packages/dd-trace/src/appsec/waf/waf_manager.js index 6a0ff1bec4e..a70f10374a9 100644 --- a/packages/dd-trace/src/appsec/waf/waf_manager.js +++ b/packages/dd-trace/src/appsec/waf/waf_manager.js @@ -16,7 +16,7 @@ class WAFManager { this.rulesVersion = this.ddwaf.diagnostics.ruleset_version this.defaultRules = rules - Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, true) + Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules, true) } _loadDDWAF (rules) { From 1987802e56b59ac0f43025149cc0896366a794ab Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 30 May 2025 07:15:32 +0200 Subject: [PATCH 16/22] Revert tag deprecation --- .../dd-trace/test/appsec/reporter.spec.js | 21 ++++++++++++--- .../dd-trace/test/appsec/waf/index.spec.js | 26 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index b5c5723d260..b8ee741b9ea 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -133,21 +133,33 @@ describe('reporter', () => { describe('reportWafInit', () => { const wafVersion = '0.0.1' const rulesVersion = '0.0.2' + const diagnosticsRules = { + loaded: ['1', '3', '4'], + failed: ['2'], + errors: { error: 'error parsing rule 2' } + } it('should add some entries to metricsQueue', () => { - Reporter.reportWafInit(wafVersion, rulesVersion, true) + Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true) expect(Reporter.metricsQueue.get('_dd.appsec.waf.version')).to.be.eq(wafVersion) + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.eq(3) + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.eq(1) + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.errors')) + .to.be.eq(JSON.stringify(diagnosticsRules.errors)) }) it('should not add entries to metricsQueue with success false', () => { Reporter.reportWafInit(wafVersion, rulesVersion, false) expect(Reporter.metricsQueue.get('_dd.appsec.waf.version')).to.be.undefined + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.undefined + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.undefined + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.errors')).to.be.undefined }) it('should call incrementWafInitMetric', () => { - Reporter.reportWafInit(wafVersion, rulesVersion, true) + Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true) expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true) }) @@ -155,10 +167,13 @@ describe('reporter', () => { it('should not fail with undefined arguments', () => { const wafVersion = undefined const rulesVersion = undefined + const diagnosticsRules = undefined - Reporter.reportWafInit(wafVersion, rulesVersion, true) + Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true) expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true) + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.eq(0) + expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.eq(0) }) }) diff --git a/packages/dd-trace/test/appsec/waf/index.spec.js b/packages/dd-trace/test/appsec/waf/index.spec.js index 8f7b717ca6d..00885fcc322 100644 --- a/packages/dd-trace/test/appsec/waf/index.spec.js +++ b/packages/dd-trace/test/appsec/waf/index.spec.js @@ -68,6 +68,7 @@ describe('WAF Manager', () => { expect(Reporter.reportWafInit).to.have.been.calledWithExactly( '1.2.3', '1.0.0', + { loaded: ['rule_1'], failed: [] }, true ) }) @@ -90,6 +91,31 @@ describe('WAF Manager', () => { waf.init(rules, config.appsec) expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.waf.version', '1.2.3') + expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.loaded', 1) + expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.error_count', 0) + expect(Reporter.metricsQueue.set).not.to.have.been.calledWith('_dd.appsec.event_rules.errors') + }) + + it('should set init metrics with errors', () => { + DDWAF.version.returns('2.3.4') + DDWAF.prototype.diagnostics = { + rules: { + loaded: ['rule_1'], + failed: ['rule_2', 'rule_3'], + errors: { + error_1: ['invalid_1'], + error_2: ['invalid_2', 'invalid_3'] + } + } + } + + waf.init(rules, config.appsec) + + expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.waf.version', '2.3.4') + expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.loaded', 1) + expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.error_count', 2) + expect(Reporter.metricsQueue.set).to.have.been.calledWithExactly('_dd.appsec.event_rules.errors', + '{"error_1":["invalid_1"],"error_2":["invalid_2","invalid_3"]}') }) }) From ac9b1d69be8776bd999abc949b4522ce7aeb6f7e Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 30 May 2025 07:47:45 +0200 Subject: [PATCH 17/22] Clean up --- packages/dd-trace/src/appsec/waf/index.js | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/packages/dd-trace/src/appsec/waf/index.js b/packages/dd-trace/src/appsec/waf/index.js index bf92a903e8c..630d6aaedca 100644 --- a/packages/dd-trace/src/appsec/waf/index.js +++ b/packages/dd-trace/src/appsec/waf/index.js @@ -33,20 +33,6 @@ function destroy () { waf.disposeContext = noop } -/* -function update (newRules) { - // TODO: check race conditions between Appsec enable/disable and WAF updates, the whole RC state management in general - if (!waf.wafManager) throw new Error('Cannot update disabled WAF') - - try { - waf.wafManager.update(newRules) - } catch (err) { - log.error('[ASM] Could not apply rules from remote config') - throw err - } -} -*/ - function run (data, req, raspRule) { if (!req) { const store = storage('legacy').getStore() From c5841ab1126d652c0d9b230342547cdc1ba2804c Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 30 May 2025 07:59:43 +0200 Subject: [PATCH 18/22] Lint --- packages/dd-trace/src/appsec/rule_manager.js | 6 ++---- packages/dd-trace/test/appsec/telemetry/waf.spec.js | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 5abbe152606..7cb784c5231 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -61,10 +61,8 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { } // check asm actions - if (updateResult.success && item.product === 'ASM') { - if (item.file?.actions?.length) { - newActions.set(item.id, item.file.actions) - } + if (updateResult.success && item.product === 'ASM' && item.file?.actions?.length) { + newActions.set(item.id, item.file.actions) } } catch (e) { wafUpdatedSuccess = false diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index 6fea048cfb1..bd7fd54c656 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -275,7 +275,7 @@ describe('Appsec Waf Telemetry metrics', () => { expect(count).to.have.been.calledOnceWithExactly('waf.config_errors', { waf_version: wafVersion, - event_rules_version: rulesVersion, + event_rules_version: rulesVersion }) expect(inc).to.have.been.calledOnce }) From 479c2e998a9541dedc1b530fe071fcd4a5582703 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Fri, 30 May 2025 09:49:47 +0200 Subject: [PATCH 19/22] Fix assertion --- integration-tests/graphql.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/graphql.spec.js b/integration-tests/graphql.spec.js index 4a66f163a83..0e7285083d8 100644 --- a/integration-tests/graphql.spec.js +++ b/integration-tests/graphql.spec.js @@ -112,7 +112,7 @@ describe('graphql', () => { assert.property(payload[1][0].metrics, '_dd.appsec.waf.duration') assert.propertyVal(payload[1][0].meta, 'appsec.event', 'true') assert.property(payload[1][0].meta, '_dd.appsec.json') - assert.propertyVal(payload[1][0].meta, '_dd.appsec.json', JSON.stringify(result)) + assert.deepStrictEqual(JSON.parse(payload[1][0].meta['_dd.appsec.json']), result) }) await axios({ From 66e1b28ebae24e11a556884eeab73ed1a95cefb8 Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Sun, 1 Jun 2025 10:37:52 +0200 Subject: [PATCH 20/22] Improve RC product check --- packages/dd-trace/src/appsec/rule_manager.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 7cb784c5231..1c456d962dc 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -7,6 +7,8 @@ const Reporter = require('./reporter') const blocking = require('./blocking') +const ASM_PRODUCTS = new Set(['ASM', 'ASM_DD', 'ASM_DATA']) + let appliedActions = new Map() function loadRules (config) { @@ -26,7 +28,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { let wafUpdatedSuccess = true for (const item of toUnapply) { - if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue + if (!ASM_PRODUCTS.has(item.product)) continue try { waf.wafManager.remove(item.path) @@ -45,7 +47,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) { } for (const item of [...toApply, ...toModify]) { - if (!['ASM_DD', 'ASM_DATA', 'ASM'].includes(item.product)) continue + if (!ASM_PRODUCTS.has(item.product)) continue try { const updateResult = waf.wafManager.update(item.product, item.file, item.path) From 1a2ebc9f46a0c78d3a24805e5cda5a1d986d56ee Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Sun, 1 Jun 2025 10:49:27 +0200 Subject: [PATCH 21/22] Avoid checking obj keys in rule manager extract errors --- packages/dd-trace/src/appsec/rule_manager.js | 7 ++++-- .../dd-trace/test/appsec/rule_manager.spec.js | 25 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 1c456d962dc..cd5ef881d5c 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -117,19 +117,22 @@ function extractErrors (obj) { if (typeof obj !== 'object' || obj === null) return null const result = {} + let isResultPopulated = false for (const [key, value] of Object.entries(obj)) { if (key === 'error' || key === 'errors') { result[key] = value + isResultPopulated = true } else if (typeof value === 'object' && value !== null) { const child = extractErrors(value) - if (child && Object.keys(child).length > 0) { + if (child) { + isResultPopulated = true result[key] = child } } } - return Object.keys(result).length > 0 ? result : null + return isResultPopulated ? result : null } function clearAllRules () { diff --git a/packages/dd-trace/test/appsec/rule_manager.spec.js b/packages/dd-trace/test/appsec/rule_manager.spec.js index d4f524a7cc5..4d6c656a467 100644 --- a/packages/dd-trace/test/appsec/rule_manager.spec.js +++ b/packages/dd-trace/test/appsec/rule_manager.spec.js @@ -257,13 +257,34 @@ describe('AppSec Rule Manager', () => { loaded: ['http-endpoint-fingerprint'], failed: [], skipped: [], + errors: { + 'missing mapping definition': [ + 'http-endpoint-fingerprint' + ] + }, warnings: { 'no mappings defined': [ 'http-endpoint-fingerprint' ] } + }, + scanners: { + error: 'Fatal error' } } + + const expectedApplyError = { + rules: { + errors: diagnostics.rules.errors + }, + processors: { + errors: diagnostics.processors.errors + }, + scanners: { + error: diagnostics.scanners.error + } + } + WAFManager.prototype.update.returns({ success: false, diagnostics }) const { toModify, toApply } = getRcConfigs() @@ -271,9 +292,9 @@ describe('AppSec Rule Manager', () => { RuleManager.updateWafFromRC({ toUnapply: [], toApply, toModify }) assert.strictEqual(toApply[0].apply_state, ERROR) - assert.strictEqual(toApply[0].apply_error, JSON.stringify({ rules: { errors: diagnostics.rules.errors } })) + assert.strictEqual(toApply[0].apply_error, JSON.stringify(expectedApplyError)) assert.strictEqual(toModify[0].apply_state, ERROR) - assert.strictEqual(toModify[0].apply_error, JSON.stringify({ rules: { errors: diagnostics.rules.errors } })) + assert.strictEqual(toModify[0].apply_error, JSON.stringify(expectedApplyError)) }) it('should report successful waf update', () => { From 079af0ef0b7125d0011e7f209260b5f02a72e46c Mon Sep 17 00:00:00 2001 From: CarlesDD Date: Sun, 1 Jun 2025 11:03:47 +0200 Subject: [PATCH 22/22] Abstract log message publish in reporter --- packages/dd-trace/src/appsec/reporter.js | 69 ++++++++++++------------ 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index e53d6d05c24..e9bdb0ff524 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -242,48 +242,49 @@ function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}, success incrementWafInitMetric(wafVersion, rulesVersion, success) } +function logWafDiagnosticMessage (product, rcConfigId, configKey, message, level) { + telemetryLogCh.publish({ + message, + level, + tags: { + log_type: `rc::${product.toLowerCase()}::diagnostic`, + appsec_config_key: configKey, + rc_config_id: rcConfigId + } + }) +} + function reportSuccessfulWafUpdate (product, rcConfigId, diagnostics) { for (const configKey of WAF_DIAGNOSTICS_CONFIG_KEYS_TO_REPORT) { - if (!diagnostics[configKey]) continue - - if (diagnostics[configKey].error) { - telemetryLogCh.publish({ - message: diagnostics[configKey].error, - level: 'ERROR', - tags: { - log_type: `rc::${product.toLowerCase()}::diagnostic`, - appsec_config_key: configKey, - rc_config_id: rcConfigId - } - }) + const configDiagnostics = diagnostics[configKey] + if (!configDiagnostics) continue + + if (configDiagnostics.error) { + logWafDiagnosticMessage(product, rcConfigId, configKey, configDiagnostics.error, 'ERROR') continue } - if (diagnostics[configKey].errors) { - for (const [errorMessage, errorIds] of Object.entries(diagnostics[configKey].errors)) { - telemetryLogCh.publish({ - message: `"${errorMessage}": ${JSON.stringify(errorIds)}`, - level: 'ERROR', - tags: { - log_type: `rc::${product.toLowerCase()}::diagnostic`, - appsec_config_key: configKey, - rc_config_id: rcConfigId - } - }) + if (configDiagnostics.errors) { + for (const [errorMessage, errorIds] of Object.entries(configDiagnostics.errors)) { + logWafDiagnosticMessage( + product, + rcConfigId, + configKey, + `"${errorMessage}": ${JSON.stringify(errorIds)}`, + 'ERROR' + ) } } - if (diagnostics[configKey].warnings) { - for (const [warningMessage, warningIds] of Object.entries(diagnostics[configKey].warnings)) { - telemetryLogCh.publish({ - message: `"${warningMessage}": ${JSON.stringify(warningIds)}`, - level: 'WARN', - tags: { - log_type: `rc::${product.toLowerCase()}::diagnostic`, - appsec_config_key: configKey, - rc_config_id: rcConfigId - } - }) + if (configDiagnostics.warnings) { + for (const [warningMessage, warningIds] of Object.entries(configDiagnostics.warnings)) { + logWafDiagnosticMessage( + product, + rcConfigId, + configKey, + `"${warningMessage}": ${JSON.stringify(warningIds)}`, + 'WARN' + ) } } }