fix: Change obfuscator to read directly from obfuscation rules configuration (#1327)

metal-messiah · web-flow · commit a75f935d9b04 · 2025-01-14T23:04:02.000Z
diff --git a/src/common/util/obfuscate.js b/src/common/util/obfuscate.js
@@ -1,4 +1,3 @@
-import { getConfigurationValue } from '../config/init'
 import { isFileProtocol } from '../url/protocol'
 import { warn } from './console'
 
@@ -21,18 +20,15 @@ import { warn } from './console'
  */
 
 export class Obfuscator {
-  /**
-   * @type {ObfuscationRuleValidation[]}
-   */
-  #ruleValidationCache
-
-  constructor (agentIdentifier) {
-    this.#ruleValidationCache = Obfuscator.getRuleValidationCache(agentIdentifier)
-    Obfuscator.logObfuscationRuleErrors(this.#ruleValidationCache)
+  constructor (agentRef) {
+    this.agentRef = agentRef
+    this.warnedRegexMissing = false
+    this.warnedInvalidRegex = false
+    this.warnedInvalidReplacement = false
   }
 
-  get ruleValidationCache () {
-    return this.#ruleValidationCache
+  get obfuscateConfigRules () {
+    return this.agentRef.init.obfuscate || []
   }
 
   /**
@@ -44,44 +40,44 @@ export class Obfuscator {
     // if input is not of type string or is an empty string, short-circuit
     if (typeof input !== 'string' || input.trim().length === 0) return input
 
-    return this.#ruleValidationCache
-      .filter(ruleValidation => ruleValidation.isValid)
-      .reduce((input, ruleValidation) => {
-        const { rule } = ruleValidation
-        return input.replace(rule.regex, rule.replacement || '*')
-      }, input)
-  }
-
-  /**
-   * Returns an array of obfuscation rules to be applied to harvested payloads
-   * @param {string} agentIdentifier The agent identifier to get rules for
-   * @returns {ObfuscationRuleValidation[]} The array of rules or validation states
-   */
-  static getRuleValidationCache (agentIdentifier) {
-    /**
-     * @type {ObfuscationRule[]}
-     */
-    let rules = getConfigurationValue(agentIdentifier, 'obfuscate') || []
+    const rules = (this.obfuscateConfigRules).map(rule => this.validateObfuscationRule(rule))
     if (isFileProtocol()) {
       rules.push({
         regex: /^file:\/\/(.*)/,
         replacement: atob('ZmlsZTovL09CRlVTQ0FURUQ=')
       })
     }
 
-    return rules.map(rule => Obfuscator.validateObfuscationRule(rule))
+    return rules
+      .filter(ruleValidation => ruleValidation.isValid)
+      .reduce((input, ruleValidation) => {
+        const { rule } = ruleValidation
+        return input.replace(rule.regex, rule.replacement || '*')
+      }, input)
   }
 
   /**
    * Validates an obfuscation rule and provides errors if any are found.
    * @param {ObfuscationRule} rule The rule to validate
    * @returns {ObfuscationRuleValidation} The validation state of the rule
    */
-  static validateObfuscationRule (rule) {
+  validateObfuscationRule (rule) {
     const regexMissingDetected = Boolean(rule.regex === undefined)
     const invalidRegexDetected = Boolean(rule.regex !== undefined && typeof rule.regex !== 'string' && !(rule.regex instanceof RegExp))
     const invalidReplacementDetected = Boolean(rule.replacement && typeof rule.replacement !== 'string')
 
+    if (regexMissingDetected && !this.warnedRegexMissing) {
+      warn(12, rule)
+      this.warnedRegexMissing = true
+    } else if (invalidRegexDetected && !this.warnedInvalidRegex) {
+      warn(13, rule)
+      this.warnedInvalidRegex = true
+    }
+    if (invalidReplacementDetected && !this.warnedInvalidReplacement) {
+      warn(14, rule)
+      this.warnedInvalidReplacement = true
+    }
+
     return {
       rule,
       isValid: !regexMissingDetected && !invalidRegexDetected && !invalidReplacementDetected,
@@ -92,20 +88,4 @@ export class Obfuscator {
       }
     }
   }
-
-  /**
-   * Logs any obfuscation rule errors to the console. This is called when an obfuscator
-   * instance is created.
-   * @param {ObfuscationRuleValidation[]} ruleValidationCache The cache of rule validation states
-   */
-  static logObfuscationRuleErrors (ruleValidationCache) {
-    for (const ruleValidation of ruleValidationCache) {
-      const { rule, isValid, errors } = ruleValidation
-      if (isValid) continue
-
-      if (errors.regexMissingDetected) warn(12, rule)
-      else if (errors.invalidRegexDetected) warn(13, rule)
-      if (errors.invalidReplacementDetected) warn(14, rule)
-    }
-  }
 }
diff --git a/src/features/metrics/aggregate/index.js b/src/features/metrics/aggregate/index.js
@@ -87,10 +87,8 @@ export class Aggregate extends AggregateBase {
     }
 
     // Capture SMs to assess customer engagement with the obfuscation config
-    const ruleValidations = this.obfuscator.ruleValidationCache
-    if (ruleValidations.length > 0) {
+    if (this.obfuscator.obfuscateConfigRules.length > 0) {
       this.storeSupportabilityMetrics('Generic/Obfuscate/Detected')
-      if (ruleValidations.filter(ruleValidation => !ruleValidation.isValid).length > 0) this.storeSupportabilityMetrics('Generic/Obfuscate/Invalid')
     }
 
     // Check if proxy for either chunks or beacon is being used
diff --git a/src/features/utils/aggregate-base.js b/src/features/utils/aggregate-base.js
@@ -142,7 +142,7 @@ export class AggregateBase extends FeatureBase {
    * This method should run after checkConfiguration, which may reset the agent's info/runtime object that is used here.
    */
   doOnceForAllAggregate (agentRef) {
-    if (!agentRef.runtime.obfuscator) agentRef.runtime.obfuscator = new Obfuscator(this.agentIdentifier)
+    if (!agentRef.runtime.obfuscator) agentRef.runtime.obfuscator = new Obfuscator(agentRef)
     this.obfuscator = agentRef.runtime.obfuscator
 
     if (!agentRef.mainAppKey) agentRef.mainAppKey = { licenseKey: agentRef.info.licenseKey, appId: agentRef.info.applicationID }
diff --git a/tests/components/spa/serializer.test.js b/tests/components/spa/serializer.test.js
@@ -43,7 +43,7 @@ const fieldPropMap = {
 beforeEach(() => {
   jest.mocked(runtimeModule.getRuntime).mockReturnValue({
     origin: 'localhost',
-    obfuscator: new Obfuscator(agentIdentifier)
+    obfuscator: new Obfuscator({ init: { obfuscate: [] } })
   })
 })
 
diff --git a/tests/specs/metrics.e2e.js b/tests/specs/metrics.e2e.js
@@ -95,7 +95,7 @@ describe('metrics', () => {
     }]))
   })
 
-  it('should create SMs for valid obfuscation rules', async () => {
+  it('should create SMs when obfuscation rules are detected', async () => {
     await browser.url(await browser.testHandle.assetURL('obfuscate-pii-valid.html'))
       .then(() => browser.waitForAgentLoad())
 
@@ -109,69 +109,5 @@ describe('metrics', () => {
       params: { name: 'Generic/Obfuscate/Detected' },
       stats: { c: 1 }
     }]))
-    expect(supportabilityMetrics).not.toEqual(expect.arrayContaining([{
-      params: { name: 'Generic/Obfuscate/Invalid' },
-      stats: { c: 1 }
-    }]))
-  })
-
-  it('should create SMs for obfuscation rule containing invalid regex type', async () => {
-    await browser.url(await browser.testHandle.assetURL('obfuscate-pii-invalid-regex-type.html'))
-      .then(() => browser.waitForAgentLoad())
-
-    const [supportabilityMetricsHarvests] = await Promise.all([
-      supportabilityMetricsCapture.waitForResult({ totalCount: 1 }),
-      await browser.url(await browser.testHandle.assetURL('/')) // Setup expects before navigating
-    ])
-
-    const supportabilityMetrics = supportabilityMetricsHarvests[0].request.body.sm
-    expect(supportabilityMetrics).toEqual(expect.arrayContaining([{
-      params: { name: 'Generic/Obfuscate/Detected' },
-      stats: { c: 1 }
-    }]))
-    expect(supportabilityMetrics).toEqual(expect.arrayContaining([{
-      params: { name: 'Generic/Obfuscate/Invalid' },
-      stats: { c: 1 }
-    }]))
-  })
-
-  it('should create SMs for obfuscation rule containing undefined regex type', async () => {
-    await browser.url(await browser.testHandle.assetURL('obfuscate-pii-invalid-regex-undefined.html'))
-      .then(() => browser.waitForAgentLoad())
-
-    const [supportabilityMetricsHarvests] = await Promise.all([
-      supportabilityMetricsCapture.waitForResult({ totalCount: 1 }),
-      await browser.url(await browser.testHandle.assetURL('/')) // Setup expects before navigating
-    ])
-
-    const supportabilityMetrics = supportabilityMetricsHarvests[0].request.body.sm
-    expect(supportabilityMetrics).toEqual(expect.arrayContaining([{
-      params: { name: 'Generic/Obfuscate/Detected' },
-      stats: { c: 1 }
-    }]))
-    expect(supportabilityMetrics).toEqual(expect.arrayContaining([{
-      params: { name: 'Generic/Obfuscate/Invalid' },
-      stats: { c: 1 }
-    }]))
-  })
-
-  it('should create SMs for obfuscation rule containing invalid replacement type', async () => {
-    await browser.url(await browser.testHandle.assetURL('obfuscate-pii-invalid-replacement-type.html'))
-      .then(() => browser.waitForAgentLoad())
-
-    const [supportabilityMetricsHarvests] = await Promise.all([
-      supportabilityMetricsCapture.waitForResult({ totalCount: 1 }),
-      await browser.url(await browser.testHandle.assetURL('/')) // Setup expects before navigating
-    ])
-
-    const supportabilityMetrics = supportabilityMetricsHarvests[0].request.body.sm
-    expect(supportabilityMetrics).toEqual(expect.arrayContaining([{
-      params: { name: 'Generic/Obfuscate/Detected' },
-      stats: { c: 1 }
-    }]))
-    expect(supportabilityMetrics).toEqual(expect.arrayContaining([{
-      params: { name: 'Generic/Obfuscate/Invalid' },
-      stats: { c: 1 }
-    }]))
   })
 })
diff --git a/tests/specs/obfuscate.e2e.js b/tests/specs/obfuscate.e2e.js
@@ -95,14 +95,46 @@ describe('obfuscate rules', () => {
       checkPayload(harvest.request.query)
     })
   })
+
+  it('should apply to obfuscation rules added after init', async () => {
+    const [insightsHarvests] = await Promise.all([
+      insightsCapture.waitForResult({ totalCount: 1 }),
+      browser.url(await browser.testHandle.assetURL('obfuscate-pii.html', config))
+        .then(() => browser.waitForAgentLoad())
+    ])
+
+    expect(insightsHarvests.length).toBeGreaterThan(0)
+    insightsHarvests.forEach(harvest => {
+      checkPayload(harvest.request.body)
+      checkPayload(harvest.request.query)
+    })
+
+    const [insightsHarvests2] = await Promise.all([
+      insightsCapture.waitForResult({ totalCount: 2 }),
+      browser.execute(function () {
+        Object.values(newrelic.initializedAgents)[0].init.obfuscate.push({
+          regex: 'newrule',
+          replacement: 'OBFUSCATED'
+        })
+        window.newrelic.addPageAction('my newrule pageaction', { foo: 'bar' })
+      })
+    ])
+
+    expect(insightsHarvests2.length).toBeGreaterThan(0)
+    insightsHarvests2.forEach(harvest => {
+      checkPayload(harvest.request.body, 'newrule')
+      checkPayload(harvest.request.query, 'newrule')
+    })
+  })
 })
 
-function checkPayload (payload) {
+function checkPayload (payload, customStringCheck) {
   expect(payload).toBeDefined() // payload exists
 
   var strPayload = JSON.stringify(payload)
 
   expect(strPayload.includes('pii')).toBeFalsy() // pii was obfuscated
   expect(strPayload.includes('bam-test')).toBeFalsy() // bam-test was obfuscated
   expect(strPayload.includes('fakeid')).toBeFalsy() // fakeid was obfuscated
+  if (customStringCheck) expect(strPayload.includes(customStringCheck)).toBeFalsy()
 }
diff --git a/tests/unit/common/util/obfuscate.test.js b/tests/unit/common/util/obfuscate.test.js
diff --git a/tests/unit/features/soft_navigations/aggregate/initial-page-load-interaction.test.js b/tests/unit/features/soft_navigations/aggregate/initial-page-load-interaction.test.js
diff --git a/tests/unit/features/soft_navigations/aggregate/interaction.test.js b/tests/unit/features/soft_navigations/aggregate/interaction.test.js

Original file line number	Diff line number	Diff line change
`@@ -87,10 +87,8 @@ export class Aggregate extends AggregateBase {`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`// Capture SMs to assess customer engagement with the obfuscation config`
`90`		`- const ruleValidations = this.obfuscator.ruleValidationCache`
`91`		`- if (ruleValidations.length > 0) {`
	`90`	`+ if (this.obfuscator.obfuscateConfigRules.length > 0) {`
`92`	`91`	`this.storeSupportabilityMetrics('Generic/Obfuscate/Detected')`
`93`		`- if (ruleValidations.filter(ruleValidation => !ruleValidation.isValid).length > 0) this.storeSupportabilityMetrics('Generic/Obfuscate/Invalid')`
`94`	`92`	`}`
`95`	`93`
`96`	`94`	`// Check if proxy for either chunks or beacon is being used`