From e590c53c05f69868e23593c61e9e067316273fe5 Mon Sep 17 00:00:00 2001
From: Gustavo Caso <gustavo.caso@datadoghq.com>
Date: Thu, 9 Mar 2023 15:29:33 +0100
Subject: [PATCH 1/3] Add AppSec::Processor::RuleMerger

---
 lib/datadog/appsec/processor/rule_merger.rb   | 107 ++++
 sig/datadog/appsec/processor/rule_merger.rbs  |  21 +
 .../appsec/processor/rule_merger_spec.rb      | 473 ++++++++++++++++++
 3 files changed, 601 insertions(+)
 create mode 100644 lib/datadog/appsec/processor/rule_merger.rb
 create mode 100644 sig/datadog/appsec/processor/rule_merger.rbs
 create mode 100644 spec/datadog/appsec/processor/rule_merger_spec.rb

diff --git a/lib/datadog/appsec/processor/rule_merger.rb b/lib/datadog/appsec/processor/rule_merger.rb
new file mode 100644
index 00000000000..684299dbd30
--- /dev/null
+++ b/lib/datadog/appsec/processor/rule_merger.rb
@@ -0,0 +1,107 @@
+# frozen_string_literal: true
+
+module Datadog
+  module AppSec
+    class Processor
+      # RuleMerger merge different sources of information
+      # into the rules payload
+      module RuleMerger
+        class << self
+          def merge(rules:, data: nil, overrides: nil)
+            rules_dup = rules.dup
+
+            rules_data = combine_data(data) if data
+            rules_overrides_and_exclusions = combine_overrides(overrides) if overrides
+
+            rules_dup.merge!(rules_data) if rules_data
+            rules_dup.merge!(rules_overrides_and_exclusions) if rules_overrides_and_exclusions
+            rules_dup
+          end
+
+          private
+
+          def combine_data(data)
+            result = []
+
+            data.each do |data_entry|
+              data_entry['rules_data'].each do |value|
+                data_exists = result.select { |x| x['id'] == value['id'] }
+
+                if data_exists.any?
+                  existing_data = data_exists.first
+
+                  if existing_data['type'] == value['type']
+                    # Duplicate entry base on type and id
+                    # We need to merge the existing data with the new one
+                    # and make sure to remove duplicates
+                    merged_data = merge_data_base_on_expiration(existing_data['data'], value['data'])
+                    existing_data['data'] = merged_data
+                  else
+                    result << value
+                  end
+                else
+                  # First entry for that id
+                  result << value
+                end
+              end
+            end
+
+            return unless result.any?
+
+            { 'rules_data' => result }
+          end
+
+          def merge_data_base_on_expiration(data1, data2)
+            result = data1.each_with_object({}) do |value, acc|
+              acc[value['value']] = value['expiration']
+            end
+
+            data2.each do |data|
+              if result.key?(data['value'])
+                # The value is duplicated so we need to keep
+                # the one with the highest expiration value
+                # We replace it if the expiration is higher than the current one
+                # or if no experiration
+                expiration = result[data['value']]
+                result[data['value']] = data['expiration'] if data['expiration'].nil? || data['expiration'] > expiration
+              else
+                result[data['value']] = data['expiration']
+              end
+            end
+
+            result.each_with_object([]) do |entry, acc|
+              # There could be cases that there is no experitaion value.
+              # To singal that there is no expiration we use the default value 0.
+              acc << { 'value' => entry[0], 'expiration' => entry[1] || 0 }
+            end
+          end
+
+          def combine_overrides(overrides)
+            result = {}
+            exclusions = []
+            rules_override = []
+
+            overrides.each do |override|
+              if override['rules_override']
+                override['rules_override'].each do |rule_override|
+                  rules_override << rule_override
+                end
+              elsif override['exclusions']
+                override['exclusions'].each do |exclusion|
+                  exclusions << exclusion
+                end
+              end
+            end
+
+            result['exclusions'] = exclusions if exclusions.any?
+            result['rules_override'] = rules_override if rules_override.any?
+
+            return if result.empty?
+
+            result
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/sig/datadog/appsec/processor/rule_merger.rbs b/sig/datadog/appsec/processor/rule_merger.rbs
new file mode 100644
index 00000000000..f1bfe3ea1c6
--- /dev/null
+++ b/sig/datadog/appsec/processor/rule_merger.rbs
@@ -0,0 +1,21 @@
+module Datadog
+  module AppSec
+    class Processor
+      module RuleMerger
+        type rules = ::Hash[::String, untyped]
+        type data = ::Hash[::String, untyped]
+        type overrides = ::Hash[::String, untyped]
+
+        def self.merge: (rules: untyped, ?data: ::Array[data]?, ?overrides: ::Array[overrides]?) -> rules
+
+        private
+
+        def self.combine_data: (::Array[data] data) -> (nil | { rules_data: untyped })
+
+        def self.merge_data_base_on_expiration: (::Array[data] data1, ::Array[data] data2) -> ::Array[data]
+
+        def self.combine_overrides: (::Array[overrides] overrides) -> overrides?
+      end
+    end
+  end
+end
diff --git a/spec/datadog/appsec/processor/rule_merger_spec.rb b/spec/datadog/appsec/processor/rule_merger_spec.rb
new file mode 100644
index 00000000000..68c0f5d1643
--- /dev/null
+++ b/spec/datadog/appsec/processor/rule_merger_spec.rb
@@ -0,0 +1,473 @@
+require 'datadog/appsec/spec_helper'
+require 'datadog/appsec/processor/rule_merger'
+
+RSpec.describe Datadog::AppSec::Processor::RuleMerger do
+  let(:rules) do
+    {
+      'version' => '2.2',
+      'metadata' => {
+        'rules_version' => '1.4.3'
+      },
+      'rules' => [
+        {
+          'id' => 'usr-001-001',
+          'name' => 'Super rule',
+          'tags' => {
+            'type' => 'security_scanner',
+            'category' => 'scanners'
+          },
+          'conditions' => [
+            {
+              'parameters' => {
+                'inputs' => [
+                  {
+                    'address' => 'server.request.headers',
+                    'key_path' => ['user-agent']
+                  }
+                ],
+                'regex' => '^SuperScanner$'
+              },
+              'operator' => 'regex_match'
+            }
+          ],
+          'transformers' => []
+        }
+      ]
+    }
+  end
+
+  context 'overrides' do
+    context 'without overrides' do
+      it 'does not merge rules_overrides or exclusions' do
+        expected_result = rules
+
+        result = described_class.merge(rules: rules)
+        expect(result).to eq(expected_result)
+      end
+    end
+
+    context 'with overrides' do
+      it 'merge rules_overrides' do
+        rules_overrides = [
+          {
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'on_match' => ['block']
+              }
+            ]
+          },
+          {
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'enabled' => false,
+              }
+            ]
+          },
+        ]
+
+        expected_result = rules.merge(
+          {
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'on_match' => ['block']
+              },
+              {
+                'id' => 'usr-001-001',
+                'enabled' => false
+              }
+            ]
+          }
+        )
+
+        result = described_class.merge(rules: rules, overrides: rules_overrides)
+        expect(result).to eq(expected_result)
+      end
+
+      it 'merges exclusions' do
+        rules_overrides = [
+          {
+            'exclusions' => [
+              {
+                'conditions' => [
+                  {
+                    'operator' => 'match_regex',
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.uri.raw'
+                        }
+                      ],
+                      'options' => {
+                        'case_sensitive' => false
+                      },
+                      'regex' => '^/api/v2/ci/pipeline/.*'
+                    }
+                  }
+                ],
+                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+              }
+            ]
+          }
+        ]
+
+        expected_result = rules.merge(
+          {
+            'exclusions' => [
+              {
+                'conditions' => [
+                  {
+                    'operator' => 'match_regex',
+                    'parameters' => {
+                      'inputs' => [
+                        { 'address' => 'server.request.uri.raw' }
+                      ],
+                      'options' => { 'case_sensitive' => false },
+                      'regex' => '^/api/v2/ci/pipeline/.*'
+                    }
+                  }
+                ],
+                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+              }
+            ]
+          }
+        )
+
+        result = described_class.merge(rules: rules, overrides: rules_overrides)
+        expect(result).to eq(expected_result)
+      end
+
+      it 'merges rules_overrides and exclusions' do
+        rules_overrides = [
+          {
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'on_match' => ['block']
+              }
+            ]
+          },
+          {
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'enabled' => false,
+              }
+            ]
+          },
+          {
+            'exclusions' => [
+              {
+                'conditions' => [
+                  {
+                    'operator' => 'match_regex',
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.uri.raw'
+                        }
+                      ],
+                      'options' => {
+                        'case_sensitive' => false
+                      },
+                      'regex' => '^/api/v2/ci/pipeline/.*'
+                    }
+                  }
+                ],
+                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+              }
+            ]
+          }
+        ]
+
+        expected_result = rules.merge(
+          {
+            'exclusions' => [
+              {
+                'conditions' => [
+                  {
+                    'operator' => 'match_regex',
+                    'parameters' => {
+                      'inputs' => [
+                        { 'address' => 'server.request.uri.raw' }
+                      ],
+                      'options' => { 'case_sensitive' => false },
+                      'regex' => '^/api/v2/ci/pipeline/.*'
+                    }
+                  }
+                ],
+                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+              }
+            ],
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'on_match' => ['block']
+              },
+              {
+                'id' => 'usr-001-001',
+                'enabled' => false
+              }
+            ]
+          }
+        )
+
+        result = described_class.merge(rules: rules, overrides: rules_overrides)
+        expect(result).to eq(expected_result)
+      end
+    end
+  end
+
+  context 'data' do
+    it 'merges rules_data' do
+      rules_data = [
+        {
+          'rules_data' => [
+            {
+              'data' => [
+                {
+                  'expiration' => 1677171437,
+                  'value' => 'this is a test'
+                }
+              ],
+              'id' => 'blocked_users',
+              'type' => 'data_with_expiration'
+            }
+          ]
+        },
+        {
+          'rules_data' => [
+            {
+              'data' => [
+                {
+                  'expiration' => 1678279317,
+                  'value' => '9.9.9.9'
+                }
+              ],
+              'id' => 'blocked_ips',
+              'type' => 'ip_with_expiration'
+            }
+          ]
+        },
+        {
+          'rules_data' => [
+            {
+              'data' => [
+                {
+                  'expiration' => 1678279317,
+                  'value' => 'this is a second test'
+                }
+              ],
+              'id' => 'blocked_users',
+              'type' => 'data_with_expiration'
+            }
+          ]
+        }
+      ]
+
+      expected_result = rules.merge(
+        {
+          'rules_data' => [
+            {
+              'id' => 'blocked_users',
+              'type' => 'data_with_expiration',
+              'data' => [
+                {
+                  'expiration' => 1677171437,
+                  'value' => 'this is a test'
+                },
+                {
+                  'expiration' => 1678279317,
+                  'value' => 'this is a second test'
+                }
+              ]
+            },
+            {
+              'id' => 'blocked_ips',
+              'type' => 'ip_with_expiration',
+              'data' => [
+                {
+                  'expiration' => 1678279317,
+                  'value' => '9.9.9.9'
+                }
+              ]
+            },
+          ]
+        }
+      )
+
+      result = described_class.merge(rules: rules, data: rules_data)
+      expect(result).to eq(expected_result)
+    end
+
+    it 'merges data of different types' do
+      rules_data = [
+        {
+          'rules_data' => [
+            {
+              'data' => [
+                {
+                  'expiration' => 1677171437,
+                  'value' => 'this is a test'
+                }
+              ],
+              'id' => 'blocked_users',
+              'type' => 'data_with_expiration'
+            }
+          ]
+        },
+        {
+          'rules_data' => [
+            {
+              'data' => [
+                {
+                  'value' => 'this is a test'
+                }
+              ],
+              'id' => 'blocked_users',
+              'type' => 'test_data'
+            }
+          ]
+        }
+      ]
+
+      expected_result = rules.merge(
+        {
+          'rules_data' => [
+            {
+              'id' => 'blocked_users',
+              'type' => 'data_with_expiration',
+              'data' => [
+                {
+                  'expiration' => 1677171437,
+                  'value' => 'this is a test'
+                }
+              ]
+            },
+            {
+              'data' => [
+                {
+                  'value' => 'this is a test'
+                }
+              ],
+              'id' => 'blocked_users',
+              'type' => 'test_data'
+            }
+          ]
+        }
+      )
+
+      result = described_class.merge(rules: rules, data: rules_data)
+      expect(result).to eq(expected_result)
+    end
+
+    context 'with duplicates entries' do
+      it 'removes duplicate entry and leave the one with the longest expiration ' do
+        rules_data = [
+          {
+            'rules_data' => [
+              {
+                'data' => [
+                  {
+                    'expiration' => 1677171437,
+                    'value' => 'this is a test'
+                  }
+                ],
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration'
+              }
+            ]
+          },
+          {
+            'rules_data' => [
+              {
+                'data' => [
+                  {
+                    'expiration' => 167710000,
+                    'value' => 'this is a test'
+                  }
+                ],
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration'
+              }
+            ]
+          }
+        ]
+
+        expected_result = rules.merge(
+          {
+            'rules_data' => [
+              {
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration',
+                'data' => [
+                  {
+                    'expiration' => 1677171437,
+                    'value' => 'this is a test'
+                  }
+                ]
+              }
+            ]
+          }
+        )
+
+        result = described_class.merge(rules: rules, data: rules_data)
+        expect(result).to eq(expected_result)
+      end
+
+      it 'keeps the entry without expiration' do
+        rules_data = [
+          {
+            'rules_data' => [
+              {
+                'data' => [
+                  {
+                    'expiration' => 1677171437,
+                    'value' => 'this is a test'
+                  }
+                ],
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration'
+              }
+            ]
+          },
+          {
+            'rules_data' => [
+              {
+                'data' => [
+                  {
+                    'value' => 'this is a test'
+                  }
+                ],
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration'
+              }
+            ]
+          }
+        ]
+
+        expected_result = rules.merge(
+          {
+            'rules_data' => [
+              {
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration',
+                'data' => [
+                  {
+                    'expiration' => 0,
+                    'value' => 'this is a test'
+                  }
+                ]
+              }
+            ]
+          }
+        )
+
+        result = described_class.merge(rules: rules, data: rules_data)
+        expect(result).to eq(expected_result)
+      end
+    end
+  end
+end

From 47b85bdb86021a194c30bb50ee7b46f6a64bbc13 Mon Sep 17 00:00:00 2001
From: Gustavo Caso <gustavo.caso@datadoghq.com>
Date: Wed, 15 Mar 2023 10:00:31 +0100
Subject: [PATCH 2/3] add support for merging rules files with same version

---
 lib/datadog/appsec/processor/rule_merger.rb   |  43 ++-
 sig/datadog/appsec/processor/rule_merger.rbs  |   8 +-
 .../appsec/processor/rule_merger_spec.rb      | 244 +++++++++++++++---
 3 files changed, 255 insertions(+), 40 deletions(-)

diff --git a/lib/datadog/appsec/processor/rule_merger.rb b/lib/datadog/appsec/processor/rule_merger.rb
index 684299dbd30..ef006df31bb 100644
--- a/lib/datadog/appsec/processor/rule_merger.rb
+++ b/lib/datadog/appsec/processor/rule_merger.rb
@@ -6,20 +6,55 @@ class Processor
       # RuleMerger merge different sources of information
       # into the rules payload
       module RuleMerger
+        # RuleVersionMismatchError
+        class RuleVersionMismatchError < StandardError
+          def initialize(version1, version2)
+            msg = 'Merging rule files with different version could lead to unkown behaviour. '\
+              "We have receieve two rule files with versions: #{version1}, #{version2}. "\
+              'Please validate the configuration is correct and try again.'
+            super(msg)
+          end
+        end
+
         class << self
           def merge(rules:, data: nil, overrides: nil)
-            rules_dup = rules.dup
+            combined_rules = combine_rules(rules)
 
             rules_data = combine_data(data) if data
             rules_overrides_and_exclusions = combine_overrides(overrides) if overrides
 
-            rules_dup.merge!(rules_data) if rules_data
-            rules_dup.merge!(rules_overrides_and_exclusions) if rules_overrides_and_exclusions
-            rules_dup
+            combined_rules.merge!(rules_data) if rules_data
+            combined_rules.merge!(rules_overrides_and_exclusions) if rules_overrides_and_exclusions
+            combined_rules
           end
 
           private
 
+          def combine_rules(rules)
+            return rules[0] if rules.size == 1
+
+            final_rules = []
+            # @type var final_version: ::String
+            final_version = (_ = nil)
+
+            rules.each do |rule_file|
+              version = rule_file['version']
+
+              if version && !final_version
+                final_version = version
+              elsif final_version != version
+                raise RuleVersionMismatchError.new(final_version, version)
+              end
+
+              final_rules.concat(rule_file['rules'])
+            end
+
+            {
+              'version' => final_version,
+              'rules' => final_rules
+            }
+          end
+
           def combine_data(data)
             result = []
 
diff --git a/sig/datadog/appsec/processor/rule_merger.rbs b/sig/datadog/appsec/processor/rule_merger.rbs
index f1bfe3ea1c6..b088002f5a6 100644
--- a/sig/datadog/appsec/processor/rule_merger.rbs
+++ b/sig/datadog/appsec/processor/rule_merger.rbs
@@ -2,14 +2,20 @@ module Datadog
   module AppSec
     class Processor
       module RuleMerger
+        class RuleVersionMismatchError < StandardError
+          def initialize: (::String version1, ::String version2) -> void
+        end
+
         type rules = ::Hash[::String, untyped]
         type data = ::Hash[::String, untyped]
         type overrides = ::Hash[::String, untyped]
 
-        def self.merge: (rules: untyped, ?data: ::Array[data]?, ?overrides: ::Array[overrides]?) -> rules
+        def self.merge: (rules: ::Array[rules], ?data: ::Array[data]?, ?overrides: ::Array[overrides]?) -> rules
 
         private
 
+        def self.combine_rules: (::Array[rules] rules) -> untyped
+
         def self.combine_data: (::Array[data] data) -> (nil | { rules_data: untyped })
 
         def self.merge_data_base_on_expiration: (::Array[data] data1, ::Array[data] data2) -> ::Array[data]
diff --git a/spec/datadog/appsec/processor/rule_merger_spec.rb b/spec/datadog/appsec/processor/rule_merger_spec.rb
index 68c0f5d1643..8b3b490d91b 100644
--- a/spec/datadog/appsec/processor/rule_merger_spec.rb
+++ b/spec/datadog/appsec/processor/rule_merger_spec.rb
@@ -3,43 +3,217 @@
 
 RSpec.describe Datadog::AppSec::Processor::RuleMerger do
   let(:rules) do
-    {
-      'version' => '2.2',
-      'metadata' => {
-        'rules_version' => '1.4.3'
-      },
-      'rules' => [
-        {
-          'id' => 'usr-001-001',
-          'name' => 'Super rule',
-          'tags' => {
-            'type' => 'security_scanner',
-            'category' => 'scanners'
-          },
-          'conditions' => [
-            {
-              'parameters' => {
-                'inputs' => [
+    [
+      {
+        'version' => '2.2',
+        'metadata' => {
+          'rules_version' => '1.4.3'
+        },
+        'rules' => [
+          {
+            'id' => 'usr-001-001',
+            'name' => 'Super rule',
+            'tags' => {
+              'type' => 'security_scanner',
+              'category' => 'scanners'
+            },
+            'conditions' => [
+              {
+                'parameters' => {
+                  'inputs' => [
+                    {
+                      'address' => 'server.request.headers',
+                      'key_path' => ['user-agent']
+                    }
+                  ],
+                  'regex' => '^SuperScanner$'
+                },
+                'operator' => 'regex_match'
+              }
+            ],
+            'transformers' => []
+          }
+        ]
+      }
+    ]
+  end
+
+  context 'rules' do
+    context 'multiple rules files' do
+      context 'version' do
+        it 'merge rules files when the version is the same' do
+          rules_dup = rules.dup
+          rules_dup[1] = {
+            'version' => '2.2',
+            'metadata' => {
+              'rules_version' => '1.4.3'
+            },
+            'rules' => [
+              {
+                'id' => 'crs-942-100',
+                'name' => 'SQL Injection Attack Detected via libinjection',
+                'tags' => {
+                  'type' => 'sql_injection',
+                  'crs_id' => '942100',
+                  'category' => 'attack_attempt'
+                },
+                'conditions' => [
                   {
-                    'address' => 'server.request.headers',
-                    'key_path' => ['user-agent']
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.query'
+                        },
+                        {
+                          'address' => 'server.request.body'
+                        },
+                        {
+                          'address' => 'server.request.path_params'
+                        },
+                        {
+                          'address' => 'grpc.server.request.message'
+                        }
+                      ]
+                    },
+                    'operator' => 'is_sqli'
                   }
                 ],
-                'regex' => '^SuperScanner$'
+                'transformers' => [
+                  'removeNulls'
+                ],
+                'on_match' => [
+                  'block'
+                ]
               },
-              'operator' => 'regex_match'
-            }
-          ],
-          'transformers' => []
-        }
-      ]
-    }
+            ]
+          }
+
+          expected_result = {
+            'version' => '2.2',
+            'rules' => [
+              {
+                'id' => 'usr-001-001',
+                'name' => 'Super rule',
+                'tags' => {
+                  'type' => 'security_scanner',
+                  'category' => 'scanners'
+                },
+                'conditions' => [
+                  {
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.headers',
+                          'key_path' => ['user-agent']
+                        }
+                      ],
+                      'regex' => '^SuperScanner$'
+                    },
+                    'operator' => 'regex_match'
+                  }
+                ],
+                'transformers' => []
+              },
+              {
+                'id' => 'crs-942-100',
+                'name' => 'SQL Injection Attack Detected via libinjection',
+                'tags' => {
+                  'type' => 'sql_injection',
+                  'crs_id' => '942100',
+                  'category' => 'attack_attempt'
+                },
+                'conditions' => [
+                  {
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.query'
+                        },
+                        {
+                          'address' => 'server.request.body'
+                        },
+                        {
+                          'address' => 'server.request.path_params'
+                        },
+                        {
+                          'address' => 'grpc.server.request.message'
+                        }
+                      ]
+                    },
+                    'operator' => 'is_sqli'
+                  }
+                ],
+                'transformers' => [
+                  'removeNulls'
+                ],
+                'on_match' => [
+                  'block'
+                ]
+              },
+            ]
+          }
+
+          result = described_class.merge(rules: rules_dup)
+          expect(result).to eq(expected_result)
+        end
+
+        it 'raises RuleVersionMismatchError is the rules version is not the same' do
+          rules_dup = rules.dup
+          rules_dup[1] = {
+            'version' => '2.3',
+            'metadata' => {
+              'rules_version' => '1.4.3'
+            },
+            'rules' => [
+              {
+                'id' => 'crs-942-100',
+                'name' => 'SQL Injection Attack Detected via libinjection',
+                'tags' => {
+                  'type' => 'sql_injection',
+                  'crs_id' => '942100',
+                  'category' => 'attack_attempt'
+                },
+                'conditions' => [
+                  {
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.query'
+                        },
+                        {
+                          'address' => 'server.request.body'
+                        },
+                        {
+                          'address' => 'server.request.path_params'
+                        },
+                        {
+                          'address' => 'grpc.server.request.message'
+                        }
+                      ]
+                    },
+                    'operator' => 'is_sqli'
+                  }
+                ],
+                'transformers' => [
+                  'removeNulls'
+                ],
+                'on_match' => [
+                  'block'
+                ]
+              },
+            ]
+          }
+
+          expect { described_class.merge(rules: rules_dup) }.to raise_error(described_class::RuleVersionMismatchError)
+        end
+      end
+    end
   end
 
   context 'overrides' do
     context 'without overrides' do
       it 'does not merge rules_overrides or exclusions' do
-        expected_result = rules
+        expected_result = rules[0]
 
         result = described_class.merge(rules: rules)
         expect(result).to eq(expected_result)
@@ -67,7 +241,7 @@
           },
         ]
 
-        expected_result = rules.merge(
+        expected_result = rules[0].merge(
           {
             'rules_override' => [
               {
@@ -113,7 +287,7 @@
           }
         ]
 
-        expected_result = rules.merge(
+        expected_result = rules[0].merge(
           {
             'exclusions' => [
               {
@@ -182,7 +356,7 @@
           }
         ]
 
-        expected_result = rules.merge(
+        expected_result = rules[0].merge(
           {
             'exclusions' => [
               {
@@ -267,7 +441,7 @@
         }
       ]
 
-      expected_result = rules.merge(
+      expected_result = rules[0].merge(
         {
           'rules_data' => [
             {
@@ -333,7 +507,7 @@
         }
       ]
 
-      expected_result = rules.merge(
+      expected_result = rules[0].merge(
         {
           'rules_data' => [
             {
@@ -396,7 +570,7 @@
           }
         ]
 
-        expected_result = rules.merge(
+        expected_result = rules[0].merge(
           {
             'rules_data' => [
               {
@@ -448,7 +622,7 @@
           }
         ]
 
-        expected_result = rules.merge(
+        expected_result = rules[0].merge(
           {
             'rules_data' => [
               {

From d1b004cf41c2ac5923880e964f8f9ba09275c9b2 Mon Sep 17 00:00:00 2001
From: Gustavo Caso <gustavo.caso@datadoghq.com>
Date: Wed, 15 Mar 2023 12:03:36 +0100
Subject: [PATCH 3/3] apply feedback

---
 lib/datadog/appsec/processor/rule_merger.rb   |  84 ++--
 sig/datadog/appsec/processor/rule_merger.rbs  |   9 +-
 .../appsec/processor/rule_merger_spec.rb      | 401 ++++++++++++------
 3 files changed, 318 insertions(+), 176 deletions(-)

diff --git a/lib/datadog/appsec/processor/rule_merger.rb b/lib/datadog/appsec/processor/rule_merger.rb
index ef006df31bb..a9b9b562770 100644
--- a/lib/datadog/appsec/processor/rule_merger.rb
+++ b/lib/datadog/appsec/processor/rule_merger.rb
@@ -17,21 +17,24 @@ def initialize(version1, version2)
         end
 
         class << self
-          def merge(rules:, data: nil, overrides: nil)
+          def merge(rules:, data: nil, overrides: nil, exclusions: nil)
             combined_rules = combine_rules(rules)
 
             rules_data = combine_data(data) if data
-            rules_overrides_and_exclusions = combine_overrides(overrides) if overrides
+            rules_overrides = combine_overrides(overrides) if overrides
+            rules_exclusions = combine_exclusions(exclusions) if exclusions
+
+            combined_rules['rules_data'] = rules_data if rules_data
+            combined_rules['rules_override'] = rules_overrides if rules_overrides
+            combined_rules['exclusions'] = rules_exclusions if rules_exclusions
 
-            combined_rules.merge!(rules_data) if rules_data
-            combined_rules.merge!(rules_overrides_and_exclusions) if rules_overrides_and_exclusions
             combined_rules
           end
 
           private
 
           def combine_rules(rules)
-            return rules[0] if rules.size == 1
+            return rules[0].dup if rules.size == 1
 
             final_rules = []
             # @type var final_version: ::String
@@ -60,22 +63,15 @@ def combine_data(data)
 
             data.each do |data_entry|
               data_entry['rules_data'].each do |value|
-                data_exists = result.select { |x| x['id'] == value['id'] }
-
-                if data_exists.any?
-                  existing_data = data_exists.first
-
-                  if existing_data['type'] == value['type']
-                    # Duplicate entry base on type and id
-                    # We need to merge the existing data with the new one
-                    # and make sure to remove duplicates
-                    merged_data = merge_data_base_on_expiration(existing_data['data'], value['data'])
-                    existing_data['data'] = merged_data
-                  else
-                    result << value
-                  end
+                existing_data = result.find { |x| x['id'] == value['id'] }
+
+                if existing_data && existing_data['type'] == value['type']
+                  # Duplicate entry base on type and id
+                  # We need to merge the existing data with the new one
+                  # and make sure to remove duplicates
+                  merged_data = merge_data_base_on_expiration(existing_data['data'], value['data'])
+                  existing_data['data'] = merged_data
                 else
-                  # First entry for that id
                   result << value
                 end
               end
@@ -83,7 +79,7 @@ def combine_data(data)
 
             return unless result.any?
 
-            { 'rules_data' => result }
+            result
           end
 
           def merge_data_base_on_expiration(data1, data2)
@@ -97,43 +93,51 @@ def merge_data_base_on_expiration(data1, data2)
                 # the one with the highest expiration value
                 # We replace it if the expiration is higher than the current one
                 # or if no experiration
-                expiration = result[data['value']]
-                result[data['value']] = data['expiration'] if data['expiration'].nil? || data['expiration'] > expiration
+                current_expiration = result[data['value']]
+                new_expiration = data['expiration']
+
+                if new_expiration.nil? || current_expiration && new_expiration > current_expiration
+                  result[data['value']] = new_expiration
+                end
               else
                 result[data['value']] = data['expiration']
               end
             end
 
             result.each_with_object([]) do |entry, acc|
-              # There could be cases that there is no experitaion value.
-              # To singal that there is no expiration we use the default value 0.
-              acc << { 'value' => entry[0], 'expiration' => entry[1] || 0 }
+              value = { 'value' => entry[0] }
+              value['expiration'] = entry[1] if entry[1]
+
+              acc << value
             end
           end
 
           def combine_overrides(overrides)
-            result = {}
-            exclusions = []
             rules_override = []
 
             overrides.each do |override|
-              if override['rules_override']
-                override['rules_override'].each do |rule_override|
-                  rules_override << rule_override
-                end
-              elsif override['exclusions']
-                override['exclusions'].each do |exclusion|
-                  exclusions << exclusion
-                end
+              override['rules_override'].each do |rule_override|
+                rules_override << rule_override
               end
             end
 
-            result['exclusions'] = exclusions if exclusions.any?
-            result['rules_override'] = rules_override if rules_override.any?
+            return if rules_override.empty?
 
-            return if result.empty?
+            rules_override
+          end
 
-            result
+          def combine_exclusions(exclusions)
+            rules_exclusions = []
+
+            exclusions.each do |exclusion|
+              exclusion['exclusions'].each do |rule_exclusion|
+                rules_exclusions << rule_exclusion
+              end
+            end
+
+            return if rules_exclusions.empty?
+
+            rules_exclusions
           end
         end
       end
diff --git a/sig/datadog/appsec/processor/rule_merger.rbs b/sig/datadog/appsec/processor/rule_merger.rbs
index b088002f5a6..3a32f3ccebd 100644
--- a/sig/datadog/appsec/processor/rule_merger.rbs
+++ b/sig/datadog/appsec/processor/rule_merger.rbs
@@ -9,18 +9,21 @@ module Datadog
         type rules = ::Hash[::String, untyped]
         type data = ::Hash[::String, untyped]
         type overrides = ::Hash[::String, untyped]
+        type exclusions = ::Hash[::String, untyped]
 
-        def self.merge: (rules: ::Array[rules], ?data: ::Array[data]?, ?overrides: ::Array[overrides]?) -> rules
+        def self.merge: (rules: ::Array[rules], ?data: ::Array[data]?, ?overrides: ::Array[overrides]?, ?exclusions: ::Array[exclusions]?) -> rules
 
         private
 
         def self.combine_rules: (::Array[rules] rules) -> untyped
 
-        def self.combine_data: (::Array[data] data) -> (nil | { rules_data: untyped })
+        def self.combine_data: (::Array[data] data) -> ::Array[data]?
 
         def self.merge_data_base_on_expiration: (::Array[data] data1, ::Array[data] data2) -> ::Array[data]
 
-        def self.combine_overrides: (::Array[overrides] overrides) -> overrides?
+        def self.combine_overrides: (::Array[overrides] overrides) -> ::Array[overrides]?
+
+        def self.combine_exclusions: (::Array[exclusions] exclusions) -> ::Array[exclusions]?
       end
     end
   end
diff --git a/spec/datadog/appsec/processor/rule_merger_spec.rb b/spec/datadog/appsec/processor/rule_merger_spec.rb
index 8b3b490d91b..f05a5804294 100644
--- a/spec/datadog/appsec/processor/rule_merger_spec.rb
+++ b/spec/datadog/appsec/processor/rule_merger_spec.rb
@@ -34,8 +34,8 @@
             'transformers' => []
           }
         ]
-      }
-    ]
+      }.freeze
+    ].freeze
   end
 
   context 'rules' do
@@ -86,7 +86,7 @@
                 ]
               },
             ]
-          }
+          }.freeze
 
           expected_result = {
             'version' => '2.2',
@@ -153,7 +153,7 @@
             ]
           }
 
-          result = described_class.merge(rules: rules_dup)
+          result = described_class.merge(rules: rules_dup.freeze)
           expect(result).to eq(expected_result)
         end
 
@@ -202,9 +202,11 @@
                 ]
               },
             ]
-          }
+          }.freeze
 
-          expect { described_class.merge(rules: rules_dup) }.to raise_error(described_class::RuleVersionMismatchError)
+          expect do
+            described_class.merge(rules: rules_dup.freeze)
+          end.to raise_error(described_class::RuleVersionMismatchError)
         end
       end
     end
@@ -259,138 +261,143 @@
         result = described_class.merge(rules: rules, overrides: rules_overrides)
         expect(result).to eq(expected_result)
       end
+    end
+  end
 
-      it 'merges exclusions' do
-        rules_overrides = [
-          {
-            'exclusions' => [
-              {
-                'conditions' => [
-                  {
-                    'operator' => 'match_regex',
-                    'parameters' => {
-                      'inputs' => [
-                        {
-                          'address' => 'server.request.uri.raw'
-                        }
-                      ],
-                      'options' => {
-                        'case_sensitive' => false
-                      },
-                      'regex' => '^/api/v2/ci/pipeline/.*'
-                    }
+  context 'exclusions' do
+    it 'merges exclusions' do
+      exclusions = [
+        {
+          'exclusions' => [
+            {
+              'conditions' => [
+                {
+                  'operator' => 'match_regex',
+                  'parameters' => {
+                    'inputs' => [
+                      {
+                        'address' => 'server.request.uri.raw'
+                      }
+                    ],
+                    'options' => {
+                      'case_sensitive' => false
+                    },
+                    'regex' => '^/api/v2/ci/pipeline/.*'
                   }
-                ],
-                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
-              }
-            ]
-          }
-        ]
-
-        expected_result = rules[0].merge(
-          {
-            'exclusions' => [
-              {
-                'conditions' => [
-                  {
-                    'operator' => 'match_regex',
-                    'parameters' => {
-                      'inputs' => [
-                        { 'address' => 'server.request.uri.raw' }
-                      ],
-                      'options' => { 'case_sensitive' => false },
-                      'regex' => '^/api/v2/ci/pipeline/.*'
-                    }
+                }
+              ],
+              'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+            }
+          ]
+        },
+        {
+          'exclusions' => [
+            {
+              'conditions' => [
+                {
+                  'operator' => 'match_regex',
+                  'parameters' => {
+                    'inputs' => [
+                      {
+                        'address' => 'server.request.uri.raw'
+                      }
+                    ],
+                    'options' => {
+                      'case_sensitive' => false
+                    },
+                    'regex' => '^/api/v2/source-code-integration/enrich-stack-trace'
                   }
-                ],
-                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
-              }
-            ]
-          }
-        )
-
-        result = described_class.merge(rules: rules, overrides: rules_overrides)
-        expect(result).to eq(expected_result)
-      end
+                }
+              ],
+              'id' => 'f40fbf52-baec-42bd-9868-cf002b6cdbed',
+              'inputs' => [
+                {
+                  'address' => 'server.request.query',
+                  'key_path' => [
+                    'stack'
+                  ]
+                },
+                {
+                  'address' => 'server.request.body',
+                  'key_path' => [
+                    'stack'
+                  ]
+                },
+                {
+                  'address' => 'server.request.path_params',
+                  'key_path' => [
+                    'stack'
+                  ]
+                }
+              ]
+            }
+          ],
+        }
+      ]
 
-      it 'merges rules_overrides and exclusions' do
-        rules_overrides = [
-          {
-            'rules_override' => [
-              {
-                'id' => 'usr-001-001',
-                'on_match' => ['block']
-              }
-            ]
-          },
-          {
-            'rules_override' => [
-              {
-                'id' => 'usr-001-001',
-                'enabled' => false,
-              }
-            ]
-          },
-          {
-            'exclusions' => [
-              {
-                'conditions' => [
-                  {
-                    'operator' => 'match_regex',
-                    'parameters' => {
-                      'inputs' => [
-                        {
-                          'address' => 'server.request.uri.raw'
-                        }
-                      ],
-                      'options' => {
-                        'case_sensitive' => false
-                      },
-                      'regex' => '^/api/v2/ci/pipeline/.*'
-                    }
+      expected_result = rules[0].merge(
+        {
+          'exclusions' => [
+            {
+              'conditions' => [
+                {
+                  'operator' => 'match_regex',
+                  'parameters' => {
+                    'inputs' => [
+                      { 'address' => 'server.request.uri.raw' }
+                    ],
+                    'options' => { 'case_sensitive' => false },
+                    'regex' => '^/api/v2/ci/pipeline/.*'
                   }
-                ],
-                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
-              }
-            ]
-          }
-        ]
-
-        expected_result = rules[0].merge(
-          {
-            'exclusions' => [
-              {
-                'conditions' => [
-                  {
-                    'operator' => 'match_regex',
-                    'parameters' => {
-                      'inputs' => [
-                        { 'address' => 'server.request.uri.raw' }
-                      ],
-                      'options' => { 'case_sensitive' => false },
-                      'regex' => '^/api/v2/ci/pipeline/.*'
-                    }
+                }
+              ],
+              'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+            },
+            {
+              'conditions' => [
+                {
+                  'operator' => 'match_regex',
+                  'parameters' => {
+                    'inputs' => [
+                      {
+                        'address' => 'server.request.uri.raw'
+                      }
+                    ],
+                    'options' => {
+                      'case_sensitive' => false
+                    },
+                    'regex' => '^/api/v2/source-code-integration/enrich-stack-trace'
                   }
-                ],
-                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
-              }
-            ],
-            'rules_override' => [
-              {
-                'id' => 'usr-001-001',
-                'on_match' => ['block']
-              },
-              {
-                'id' => 'usr-001-001',
-                'enabled' => false
-              }
-            ]
-          }
-        )
+                }
+              ],
+              'id' => 'f40fbf52-baec-42bd-9868-cf002b6cdbed',
+              'inputs' => [
+                {
+                  'address' => 'server.request.query',
+                  'key_path' => [
+                    'stack'
+                  ]
+                },
+                {
+                  'address' => 'server.request.body',
+                  'key_path' => [
+                    'stack'
+                  ]
+                },
+                {
+                  'address' => 'server.request.path_params',
+                  'key_path' => [
+                    'stack'
+                  ]
+                }
+              ]
+            }
+          ]
+        }
+      )
 
-        result = described_class.merge(rules: rules, overrides: rules_overrides)
-        expect(result).to eq(expected_result)
-      end
+      result = described_class.merge(rules: rules, exclusions: exclusions)
+      expect(result).to eq(expected_result)
     end
   end
 
@@ -591,7 +598,7 @@
         expect(result).to eq(expected_result)
       end
 
-      it 'keeps the entry without expiration' do
+      it 'removes expiration key if no experation is provided' do
         rules_data = [
           {
             'rules_data' => [
@@ -630,7 +637,6 @@
                 'type' => 'data_with_expiration',
                 'data' => [
                   {
-                    'expiration' => 0,
                     'value' => 'this is a test'
                   }
                 ]
@@ -643,5 +649,134 @@
         expect(result).to eq(expected_result)
       end
     end
+
+    context 'data, overrides, and exclusions' do
+      it 'merges all information' do
+        rules_data = [
+          {
+            'rules_data' => [
+              {
+                'data' => [
+                  {
+                    'expiration' => 1677171437,
+                    'value' => 'this is a test'
+                  }
+                ],
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration'
+              }
+            ]
+          },
+          {
+            'rules_data' => [
+              {
+                'data' => [
+                  {
+                    'value' => 'this is a test'
+                  }
+                ],
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration'
+              }
+            ]
+          }
+        ]
+
+        exclusions = [
+          {
+            'exclusions' => [
+              {
+                'conditions' => [
+                  {
+                    'operator' => 'match_regex',
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.uri.raw'
+                        }
+                      ],
+                      'options' => {
+                        'case_sensitive' => false
+                      },
+                      'regex' => '^/api/v2/ci/pipeline/.*'
+                    }
+                  }
+                ],
+                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+              }
+            ]
+          },
+        ]
+
+        rules_overrides = [
+          {
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'on_match' => ['block']
+              }
+            ]
+          },
+          {
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'enabled' => false,
+              }
+            ]
+          },
+        ]
+
+        expected_result = rules[0].merge(
+          {
+            'rules_data' => [
+              {
+                'id' => 'blocked_users',
+                'type' => 'data_with_expiration',
+                'data' => [
+                  {
+                    'value' => 'this is a test'
+                  }
+                ]
+              }
+            ],
+            'rules_override' => [
+              {
+                'id' => 'usr-001-001',
+                'on_match' => ['block']
+              },
+              {
+                'enabled' => false,
+                'id' => 'usr-001-001'
+              }
+            ],
+            'exclusions' => [
+              {
+                'conditions' => [
+                  {
+                    'operator' => 'match_regex',
+                    'parameters' => {
+                      'inputs' => [
+                        {
+                          'address' => 'server.request.uri.raw'
+                        }
+                      ],
+                      'options' => {
+                        'case_sensitive' => false
+                      },
+                      'regex' => '^/api/v2/ci/pipeline/.*'
+                    }
+                  }
+                ],
+                'id' => '1931d0f4-c521-4500-af34-6c4d8b8b3494'
+              }
+            ]
+          }
+        )
+
+        result = described_class.merge(rules: rules, data: rules_data, overrides: rules_overrides, exclusions: exclusions)
+        expect(result).to eq(expected_result)
+      end
+    end
   end
 end