Merge pull request #4173 from DataDog/appsec-fix-libddwaf-actions-han…

…dling

Fix AppSec libddwaf actions handling after update to 1.17.0.0.0

.github/workflows/system-tests.yml

@@ -220,6 +220,7 @@ jobs:
           - APPSEC_BLOCKING_FULL_DENYLIST
           - APPSEC_REQUEST_BLOCKING
           - APPSEC_STANDALONE
+          - APPSEC_BLOCKING
         include:
           - library: ruby
             app: rack

lib/datadog/appsec.rb

@@ -24,12 +24,12 @@ def processor
         appsec_component.processor if appsec_component
       end
 
-      def reconfigure(ruleset:, actions:, telemetry:)
+      def reconfigure(ruleset:, telemetry:)
         appsec_component = components.appsec
 
         return unless appsec_component
 
-        appsec_component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
+        appsec_component.reconfigure(ruleset: ruleset, telemetry: telemetry)
       end
 
       def reconfigure_lock(&block)

lib/datadog/appsec/component.rb

@@ -3,7 +3,6 @@
 require_relative 'processor'
 require_relative 'processor/rule_merger'
 require_relative 'processor/rule_loader'
-require_relative 'processor/actions'
 
 module Datadog
   module AppSec
@@ -52,10 +51,6 @@ def create_processor(settings, telemetry)
           )
           return nil unless rules
 
-          actions = rules['actions']
-
-          AppSec::Processor::Actions.merge(actions) if actions
-
           data = AppSec::Processor::RuleLoader.load_data(
             ip_denylist: settings.appsec.ip_denylist,
             user_id_denylist: settings.appsec.user_id_denylist,
@@ -84,10 +79,8 @@ def initialize(processor:)
         @mutex = Mutex.new
       end
 
-      def reconfigure(ruleset:, actions:, telemetry:)
+      def reconfigure(ruleset:, telemetry:)
         @mutex.synchronize do
-          AppSec::Processor::Actions.merge(actions)
-
           new = Processor.new(ruleset: ruleset, telemetry: telemetry)
 
           if new && new.ready?

lib/datadog/appsec/remote.rb

@@ -67,7 +67,6 @@ def receivers(telemetry)
             data = []
             overrides = []
             exclusions = []
-            actions = []
 
             repository.contents.each do |content|
               parsed_content = parse_content(content)
@@ -81,7 +80,6 @@ def receivers(telemetry)
                 overrides << parsed_content['rules_override'] if parsed_content['rules_override']
                 exclusions << parsed_content['exclusions'] if parsed_content['exclusions']
                 custom_rules << parsed_content['custom_rules'] if parsed_content['custom_rules']
-                actions.concat(parsed_content['actions']) if parsed_content['actions']
               end
             end
 
@@ -105,7 +103,7 @@ def receivers(telemetry)
               telemetry: telemetry
             )
 
-            Datadog::AppSec.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
+            Datadog::AppSec.reconfigure(ruleset: ruleset, telemetry: telemetry)
           end
 
           [receiver]

lib/datadog/appsec/response.rb

@@ -31,19 +31,16 @@ class << self
         def negotiate(env, actions)
           # @type var configured_response: Response?
           configured_response = nil
-          actions.each do |action|
+          actions.each do |type, parameters|
             # Need to use next to make steep happy :(
             # I rather use break to stop the execution
             next if configured_response
 
-            action_configuration = AppSec::Processor::Actions.fetch_configuration(action)
-            next unless action_configuration
-
-            configured_response = case action_configuration['type']
+            configured_response = case type
                                   when 'block_request'
-                                    block_response(env, action_configuration['parameters'])
+                                    block_response(env, parameters)
                                   when 'redirect_request'
-                                    redirect_response(env, action_configuration['parameters'])
+                                    redirect_response(env, parameters)
                                   end
           end
 

sig/datadog/appsec.rbs

@@ -6,7 +6,6 @@ module Datadog
 
     def self.reconfigure: (
       ruleset: ::Hash[untyped, untyped],
-      actions: Array[Hash[String, untyped]],
       telemetry: Datadog::Core::Telemetry::Component
     ) -> void
 

sig/datadog/appsec/response.rbs

@@ -11,7 +11,7 @@ module Datadog
       def to_sinatra_response: () -> ::Sinatra::Response
       def to_action_dispatch_response: () -> ::ActionDispatch::Response
 
-      def self.negotiate: (::Hash[untyped, untyped] env, Array[String] actions) -> Response
+      def self.negotiate: (::Hash[untyped, untyped] env, ::Hash[String, untyped] actions) -> Response
       def self.graphql_response: (Datadog::AppSec::Contrib::GraphQL::Gateway::Multiplex gateway_multiplex) -> Array[::GraphQL::Query::Result]
 
       private

spec/datadog/appsec/component_spec.rb

@@ -73,86 +73,6 @@
           expect(component.processor).to be_nil
         end
       end
-
-      context 'when static rules have actions defined' do
-        it 'calls Datadog::AppSec::Processor::Actions.merge' do
-          actions = [
-            {
-              'id' => 'block',
-              'type' => 'block_request',
-              'parameters' => {
-                'type' => 'auto',
-                'status_code' => 403,
-
-              }
-            }
-          ]
-          ruleset =
-            {
-              'version' => '2.2',
-              'rules' => [{
-                'conditions' => [{
-                  'operator' => 'ip_match',
-                  'parameters' => {
-                    'data' => 'blocked_ips',
-                    'inputs' => [{
-                      'address' => 'http.client_ip'
-                    }]
-                  }
-                }],
-                'id' => 'blk-001-001',
-                'name' => 'Block IP Addresses',
-                'on_match' => ['block'],
-                'tags' => {
-                  'category' => 'security_response', 'type' => 'block_ip'
-                },
-                'transformers' => []
-              }],
-              'actions' => actions
-            }
-
-          expect(Datadog::AppSec::Processor::Actions).to receive(:merge).with(actions)
-          expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(ruleset)
-
-          component = described_class.build_appsec_component(settings, telemetry: telemetry)
-
-          expect(component.processor).to be_a(Datadog::AppSec::Processor)
-        end
-      end
-
-      context 'when static rules do not  have actions defined' do
-        it 'calls Datadog::AppSec::Processor::Actions.merge' do
-          ruleset =
-            {
-              'version' => '2.2',
-              'rules' => [{
-                'conditions' => [{
-                  'operator' => 'ip_match',
-                  'parameters' => {
-                    'data' => 'blocked_ips',
-                    'inputs' => [{
-                      'address' => 'http.client_ip'
-                    }]
-                  }
-                }],
-                'id' => 'blk-001-001',
-                'name' => 'Block IP Addresses',
-                'on_match' => ['block'],
-                'tags' => {
-                  'category' => 'security_response', 'type' => 'block_ip'
-                },
-                'transformers' => []
-              }],
-            }
-
-          expect(Datadog::AppSec::Processor::Actions).to_not receive(:merge)
-          expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(ruleset)
-
-          component = described_class.build_appsec_component(settings, telemetry: telemetry)
-
-          expect(component.processor).to be_a(Datadog::AppSec::Processor)
-        end
-      end
     end
 
     context 'when appsec is not enabled' do
@@ -220,27 +140,14 @@
       }
     end
 
-    let(:actions) { [] }
-
     context 'lock' do
       it 'makes sure to synchronize' do
         mutex = Mutex.new
         processor = instance_double(Datadog::AppSec::Processor)
         component = described_class.new(processor: processor)
         component.instance_variable_set(:@mutex, mutex)
         expect(mutex).to receive(:synchronize)
-        component.reconfigure(ruleset: {}, actions: actions, telemetry: telemetry)
-      end
-    end
-
-    context 'actions' do
-      it 'merges the actions' do
-        processor = instance_double(Datadog::AppSec::Processor)
-        expect(processor).to receive(:finalize)
-        component = described_class.new(processor: processor)
-
-        expect(Datadog::AppSec::Processor::Actions).to receive(:merge).with(actions)
-        component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
+        component.reconfigure(ruleset: {}, telemetry: telemetry)
       end
     end
 
@@ -252,7 +159,7 @@
         old_processor = component.processor
 
         expect(old_processor).to receive(:finalize)
-        component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
+        component.reconfigure(ruleset: ruleset, telemetry: telemetry)
         new_processor = component.processor
         expect(new_processor).to_not eq(old_processor)
         new_processor.finalize
@@ -267,7 +174,7 @@
         old_processor = component.processor
 
         expect(old_processor).to_not receive(:finalize)
-        component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
+        component.reconfigure(ruleset: ruleset, telemetry: telemetry)
         new_processor = component.processor
         expect(new_processor).to_not eq(old_processor)
         new_processor.finalize
@@ -284,7 +191,7 @@
         ruleset = { 'invalid_one' => true }
 
         expect(old_processor).to_not receive(:finalize)
-        component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
+        component.reconfigure(ruleset: ruleset, telemetry: telemetry)
         expect(component.processor).to eq(old_processor)
       end
     end