From 53bf41a540f93bd6dd0e5bc0b1ce46a6ab47a12e Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Thu, 16 Jan 2025 21:06:27 +0100 Subject: [PATCH 1/6] Add AppSec::ActionHandler module --- lib/datadog/appsec/action_handler.rb | 36 +++++ lib/datadog/appsec/component.rb | 1 + .../appsec/contrib/rack/gateway/watcher.rb | 21 ++- .../contrib/rack/request_body_middleware.rb | 4 +- .../appsec/contrib/rack/request_middleware.rb | 4 +- .../appsec/contrib/rails/gateway/watcher.rb | 7 +- lib/datadog/appsec/contrib/rails/patcher.rb | 16 +- lib/datadog/appsec/contrib/sinatra/ext.rb | 14 -- .../appsec/contrib/sinatra/gateway/watcher.rb | 14 +- lib/datadog/appsec/contrib/sinatra/patcher.rb | 29 +--- lib/datadog/appsec/monitor/gateway/watcher.rb | 7 +- lib/datadog/appsec/response.rb | 84 +++-------- sig/datadog/appsec/action_handler.rbs | 17 +++ sig/datadog/appsec/contrib/sinatra/ext.rbs | 13 -- sig/datadog/appsec/response.rbs | 11 +- spec/datadog/appsec/response_spec.rb | 141 ++++++------------ 16 files changed, 165 insertions(+), 254 deletions(-) create mode 100644 lib/datadog/appsec/action_handler.rb delete mode 100644 lib/datadog/appsec/contrib/sinatra/ext.rb create mode 100644 sig/datadog/appsec/action_handler.rbs delete mode 100644 sig/datadog/appsec/contrib/sinatra/ext.rbs diff --git a/lib/datadog/appsec/action_handler.rb b/lib/datadog/appsec/action_handler.rb new file mode 100644 index 00000000000..6ca3663057d --- /dev/null +++ b/lib/datadog/appsec/action_handler.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + # this module encapsulates functions for handling actions that libddawf returns + module ActionHandler + module_function + + def handle(type, action_params) + case type + when 'block_request' then block_request(action_params) + when 'redirect_request' then redirect_request(action_params) + when 'generate_stack' then generate_stack(action_params) + when 'generate_schema' then generate_schema(action_params) + when 'monitor' then monitor(action_params) + else + Datadog.logger.error "Unknown action type: #{type}" + end + end + + def block_request(action_params) + throw(Datadog::AppSec::Ext::INTERRUPT, action_params) + end + + def redirect_request(action_params) + throw(Datadog::AppSec::Ext::INTERRUPT, action_params) + end + + def generate_stack(_action_params); end + + def generate_schema(_action_params); end + + def monitor(_action_params); end + end + end +end diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index 7eb46adb141..097c32e00e7 100644 --- a/lib/datadog/appsec/component.rb +++ b/lib/datadog/appsec/component.rb @@ -3,6 +3,7 @@ require_relative 'processor' require_relative 'processor/rule_merger' require_relative 'processor/rule_loader' +require_relative 'action_handler' module Datadog module AppSec diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index f3ef9e46699..4f68d746755 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -44,11 +44,14 @@ def watch_request(gateway = Instrumentation.gateway) context.trace.keep! if context.trace Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end end - block = Rack::Reactive::Request.publish(engine, gateway_request) - throw(Datadog::AppSec::Ext::INTERRUPT, event[:actions]) if block + Rack::Reactive::Request.publish(engine, gateway_request) stack.call(gateway_request.request) end @@ -75,11 +78,14 @@ def watch_response(gateway = Instrumentation.gateway) context.trace.keep! if context.trace Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end end - block = Rack::Reactive::Response.publish(engine, gateway_response) - throw(Datadog::AppSec::Ext::INTERRUPT, event[:actions]) if block + Rack::Reactive::Response.publish(engine, gateway_response) stack.call(gateway_response.response) end @@ -106,11 +112,14 @@ def watch_request_body(gateway = Instrumentation.gateway) context.trace.keep! if context.trace Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end end - block = Rack::Reactive::RequestBody.publish(engine, gateway_request) - throw(Datadog::AppSec::Ext::INTERRUPT, event[:actions]) if block + Rack::Reactive::RequestBody.publish(engine, gateway_request) stack.call(gateway_request.request) end diff --git a/lib/datadog/appsec/contrib/rack/request_body_middleware.rb b/lib/datadog/appsec/contrib/rack/request_body_middleware.rb index 3159f228e79..cb45d16328e 100644 --- a/lib/datadog/appsec/contrib/rack/request_body_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_body_middleware.rb @@ -24,7 +24,7 @@ def call(env) # TODO: handle exceptions, except for @app.call http_response = nil - block_actions = catch(::Datadog::AppSec::Ext::INTERRUPT) do + block_action_params = catch(::Datadog::AppSec::Ext::INTERRUPT) do http_response, = Instrumentation.gateway.push('rack.request.body', Gateway::Request.new(env)) do @app.call(env) end @@ -32,7 +32,7 @@ def call(env) nil end - return AppSec::Response.negotiate(env, block_actions).to_rack if block_actions + return AppSec::Response.build(block_action_params, env['HTTP_ACCEPT']).to_rack if block_action_params http_response end diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 52cfcefc420..6d380422580 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -76,7 +76,7 @@ def call(env) gateway_request = Gateway::Request.new(env) gateway_response = nil - block_actions = catch(::Datadog::AppSec::Ext::INTERRUPT) do + block_action_params = catch(::Datadog::AppSec::Ext::INTERRUPT) do http_response, = Instrumentation.gateway.push('rack.request', gateway_request) do @app.call(env) end @@ -90,7 +90,7 @@ def call(env) nil end - http_response = AppSec::Response.negotiate(env, block_actions).to_rack if block_actions + http_response = AppSec::Response.build(block_action_params, env['HTTP_ACCEPT']).to_rack if block_action_params if AppSec.api_security_enabled? ctx.events << { diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index 33228b4bf9b..44a7a3a7d2a 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -40,11 +40,14 @@ def watch_request_action(gateway = Instrumentation.gateway) context.trace.keep! if context.trace Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end end - block = Rails::Reactive::Action.publish(engine, gateway_request) - next [nil, [[:block, event]]] if block + Rails::Reactive::Action.publish(engine, gateway_request) stack.call(gateway_request.request) end diff --git a/lib/datadog/appsec/contrib/rails/patcher.rb b/lib/datadog/appsec/contrib/rails/patcher.rb index 479f65355d8..38f9a11dc7b 100644 --- a/lib/datadog/appsec/contrib/rails/patcher.rb +++ b/lib/datadog/appsec/contrib/rails/patcher.rb @@ -80,22 +80,12 @@ def process_action(*args) # TODO: handle exceptions, except for super gateway_request = Gateway::Request.new(request) - request_return, request_response = Instrumentation.gateway.push('rails.request.action', gateway_request) do - super - end - if request_response - blocked_event = request_response.find { |action, _options| action == :block } - if blocked_event - @_response = AppSec::Response.negotiate( - env, - blocked_event.last[:actions] - ).to_action_dispatch_response - request_return = @_response.body - end + http_response, = Instrumentation.gateway.push('rails.request.action', gateway_request) do + super end - request_return + http_response end end diff --git a/lib/datadog/appsec/contrib/sinatra/ext.rb b/lib/datadog/appsec/contrib/sinatra/ext.rb deleted file mode 100644 index 1002bcbb0b9..00000000000 --- a/lib/datadog/appsec/contrib/sinatra/ext.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -module Datadog - module AppSec - module Contrib - module Sinatra - # Sinatra integration constants - module Ext - ROUTE_INTERRUPT = :datadog_appsec_contrib_sinatra_route_interrupt - end - end - end - end -end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index 90f7e669e9b..f0e99a31ada 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -42,11 +42,14 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) context.trace.keep! if context.trace Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end end - block = Rack::Reactive::RequestBody.publish(engine, gateway_request) - next [nil, [[:block, event]]] if block + Rack::Reactive::RequestBody.publish(engine, gateway_request) stack.call(gateway_request.request) end @@ -73,11 +76,14 @@ def watch_request_routed(gateway = Instrumentation.gateway) context.trace.keep! if context.trace Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end end - block = Sinatra::Reactive::Routed.publish(engine, [gateway_request, gateway_route_params]) - next [nil, [[:block, event]]] if block + Sinatra::Reactive::Routed.publish(engine, [gateway_request, gateway_route_params]) stack.call(gateway_request.request) end diff --git a/lib/datadog/appsec/contrib/sinatra/patcher.rb b/lib/datadog/appsec/contrib/sinatra/patcher.rb index be4b4a73107..8b2efc56a5d 100644 --- a/lib/datadog/appsec/contrib/sinatra/patcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/patcher.rb @@ -6,7 +6,6 @@ require_relative '../../response' require_relative '../rack/request_middleware' require_relative 'framework' -require_relative 'ext' require_relative 'gateway/watcher' require_relative 'gateway/route_params' require_relative 'gateway/request' @@ -62,17 +61,8 @@ def dispatch! gateway_request = Gateway::Request.new(env) - request_return, request_response = Instrumentation.gateway.push('sinatra.request.dispatch', gateway_request) do - # handle process_route interruption - catch(Datadog::AppSec::Contrib::Sinatra::Ext::ROUTE_INTERRUPT) { super } - end - - if request_response - blocked_event = request_response.find { |action, _options| action == :block } - if blocked_event - self.response = AppSec::Response.negotiate(env, blocked_event.last[:actions]).to_sinatra_response - request_return = nil - end + request_return, = Instrumentation.gateway.push('sinatra.request.dispatch', gateway_request) do + super end request_return @@ -103,20 +93,7 @@ def process_route(*) gateway_request = Gateway::Request.new(env) gateway_route_params = Gateway::RouteParams.new(route_params) - _, request_response = Instrumentation.gateway.push( - 'sinatra.request.routed', - [gateway_request, gateway_route_params] - ) - - if request_response - blocked_event = request_response.find { |action, _options| action == :block } - if blocked_event - self.response = AppSec::Response.negotiate(env, blocked_event.last[:actions]).to_sinatra_response - - # interrupt request and return response to dispatch! for consistency - throw(Datadog::AppSec::Contrib::Sinatra::Ext::ROUTE_INTERRUPT, response) - end - end + Instrumentation.gateway.push('sinatra.request.routed', [gateway_request, gateway_route_params]) yield(*args) end diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 04595ffe52c..64a9c31c532 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -38,11 +38,14 @@ def watch_user_id(gateway = Instrumentation.gateway) context.trace.keep! if context.trace Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end end - block = Monitor::Reactive::SetUser.publish(engine, user) - throw(Datadog::AppSec::Ext::INTERRUPT, event[:actions]) if block + Monitor::Reactive::SetUser.publish(engine, user) stack.call(user) end diff --git a/lib/datadog/appsec/response.rb b/lib/datadog/appsec/response.rb index 8d63fd88622..33a4c7ccc9b 100644 --- a/lib/datadog/appsec/response.rb +++ b/lib/datadog/appsec/response.rb @@ -19,32 +19,11 @@ def to_rack [status, headers, body] end - def to_sinatra_response - ::Sinatra::Response.new(body, status, headers) - end - - def to_action_dispatch_response - ::ActionDispatch::Response.new(status, headers, body) - end - class << self - def negotiate(env, actions) - # @type var configured_response: Response? - configured_response = nil - 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 - - configured_response = case type - when 'block_request' - block_response(env, parameters) - when 'redirect_request' - redirect_response(env, parameters) - end - end + def build(action_params, http_accept_header) + return redirect_response(action_params) if action_params['location'] - configured_response || default_response(env) + block_response(action_params, http_accept_header) end def graphql_response(gateway_multiplex) @@ -63,56 +42,29 @@ def graphql_response(gateway_multiplex) private - def default_response(env) - content_type = content_type(env) - - body = [] - body << content(content_type) + def block_response(action_params, http_accept_header) + content_type = case action_params['type'] + when nil, 'auto' then content_type(http_accept_header) + else FORMAT_TO_CONTENT_TYPE.fetch(action_params['type'], DEFAULT_CONTENT_TYPE) + end Response.new( - status: 403, + status: action_params['status_code']&.to_i || 403, headers: { 'Content-Type' => content_type }, - body: body, + body: [content(content_type)], ) end - def block_response(env, options) - content_type = if options['type'] == 'auto' - content_type(env) - else - FORMAT_TO_CONTENT_TYPE[options['type']] - end - - body = [] - body << content(content_type) + def redirect_response(action_params) + status_code = action_params['status_code'].to_i Response.new( - status: options['status_code']&.to_i || 403, - headers: { 'Content-Type' => content_type }, - body: body, + status: (status_code >= 300 && status_code < 400 ? status_code : 303), + headers: { 'Location' => action_params.fetch('location') }, + body: [], ) end - def redirect_response(env, options) - if options['location'] && !options['location'].empty? - content_type = content_type(env) - - headers = { - 'Content-Type' => content_type, - 'Location' => options['location'] - } - - status_code = options['status_code'].to_i - Response.new( - status: (status_code >= 300 && status_code < 400 ? status_code : 303), - headers: headers, - body: [], - ) - else - default_response(env) - end - end - CONTENT_TYPE_TO_FORMAT = { 'application/json' => :json, 'text/html' => :html, @@ -126,10 +78,10 @@ def redirect_response(env, options) DEFAULT_CONTENT_TYPE = 'application/json' - def content_type(env) - return DEFAULT_CONTENT_TYPE unless env.key?('HTTP_ACCEPT') + def content_type(http_accept_header) + return DEFAULT_CONTENT_TYPE if http_accept_header.nil? - accept_types = env['HTTP_ACCEPT'].split(',').map(&:strip) + accept_types = http_accept_header.split(',').map(&:strip) accepted = accept_types.map { |m| Utils::HTTP::MediaRange.new(m) }.sort!.reverse! diff --git a/sig/datadog/appsec/action_handler.rbs b/sig/datadog/appsec/action_handler.rbs new file mode 100644 index 00000000000..86fab71f40d --- /dev/null +++ b/sig/datadog/appsec/action_handler.rbs @@ -0,0 +1,17 @@ +module Datadog + module AppSec + module ActionHandler + def handle: (::String type, ::Hash[::String, ::String] action_params) -> void + + def block_request: (::Hash[::String, ::String] action_params) -> void + + def redirect_request: (::Hash[::String, ::String] action_params) -> void + + def generate_stack: (::Hash[::String, ::String] action_params) -> void + + def generate_schema: (::Hash[::String, ::String] action_params) -> void + + def monitor: (::Hash[::String, ::String] action_params) -> void + end + end +end diff --git a/sig/datadog/appsec/contrib/sinatra/ext.rbs b/sig/datadog/appsec/contrib/sinatra/ext.rbs deleted file mode 100644 index a7a1912c378..00000000000 --- a/sig/datadog/appsec/contrib/sinatra/ext.rbs +++ /dev/null @@ -1,13 +0,0 @@ -module Datadog - module AppSec - module Contrib - module Sinatra - module Ext - APP: "sinatra" - - ROUTE_INTERRUPT: :datadog_appsec_contrib_sinatra_route_interrupt - end - end - end - end -end diff --git a/sig/datadog/appsec/response.rbs b/sig/datadog/appsec/response.rbs index 3d00dedc5a9..0eb5b47f757 100644 --- a/sig/datadog/appsec/response.rbs +++ b/sig/datadog/appsec/response.rbs @@ -8,10 +8,8 @@ module Datadog def initialize: (status: ::Integer, ?headers: ::Hash[::String, ::String], ?body: ::Array[::String]) -> void def to_rack: () -> ::Array[untyped] - def to_sinatra_response: () -> ::Sinatra::Response - def to_action_dispatch_response: () -> ::ActionDispatch::Response - def self.negotiate: (::Hash[untyped, untyped] env, ::Hash[String, untyped] actions) -> Response + def self.build: (::Hash[::String, ::String] action_params, ::String http_accept_header) -> Response def self.graphql_response: (Datadog::AppSec::Contrib::GraphQL::Gateway::Multiplex gateway_multiplex) -> Array[::GraphQL::Query::Result] private @@ -20,11 +18,10 @@ module Datadog FORMAT_TO_CONTENT_TYPE: ::Hash[::String, ::String] DEFAULT_CONTENT_TYPE: ::String - def self.default_response: (::Hash[untyped, untyped] env) -> Response - def self.block_response: (::Hash[untyped, untyped] env, ::Hash[String, untyped] options) -> Response - def self.redirect_response: (::Hash[untyped, untyped] env, ::Hash[String, untyped] options) -> Response + def self.block_response: (::Hash[::String, ::String] action_params, ::String http_accept_header) -> Response + def self.redirect_response: (::Hash[::String, ::String] action_params) -> Response - def self.content_type: (::Hash[untyped, untyped] env) -> ::String + def self.content_type: (::String) -> ::String def self.content: (::String) -> ::String end end diff --git a/spec/datadog/appsec/response_spec.rb b/spec/datadog/appsec/response_spec.rb index 0872ae57060..2965ca89911 100644 --- a/spec/datadog/appsec/response_spec.rb +++ b/spec/datadog/appsec/response_spec.rb @@ -1,22 +1,15 @@ require 'datadog/appsec/response' RSpec.describe Datadog::AppSec::Response do - describe '.negotiate' do - let(:env) { double } + describe '.build' do + let(:http_accept_header) { 'text/html' } - before do - allow(env).to receive(:key?).with('HTTP_ACCEPT').and_return(true) - allow(env).to receive(:[]).with('HTTP_ACCEPT').and_return('text/html') - end - - describe 'configured actions' do + describe 'configured action_params' do describe 'block' do - let(:actions) do + let(:action_params) do { - 'block_request' => { - 'type' => type, - 'status_code' => status_code - } + 'type' => type, + 'status_code' => status_code } end @@ -24,7 +17,7 @@ let(:status_code) { '100' } context 'status_code' do - subject(:status) { described_class.negotiate(env, actions).status } + subject(:status) { described_class.build(action_params, http_accept_header).status } it { is_expected.to eq 100 } @@ -36,42 +29,34 @@ end context 'body' do - subject(:body) { described_class.negotiate(env, actions).body } + subject(:body) { described_class.build(action_params, http_accept_header).body } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :html)] } context 'type is auto it uses the HTTP_ACCEPT to decide the result' do let(:type) { 'auto' } - - before do - expect(env).to receive(:key?).with('HTTP_ACCEPT').and_return(true) - expect(env).to receive(:[]).with('HTTP_ACCEPT').and_return('application/json') - end + let(:http_accept_header) { 'application/json' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :json)] } end end context 'headers' do - subject(:header) { described_class.negotiate(env, actions).headers['Content-Type'] } + subject(:header) { described_class.build(action_params, http_accept_header).headers['Content-Type'] } it { is_expected.to eq 'text/html' } context 'type is auto it uses the HTTP_ACCEPT to decide the result' do let(:type) { 'auto' } - - before do - expect(env).to receive(:key?).with('HTTP_ACCEPT').and_return(true) - expect(env).to receive(:[]).with('HTTP_ACCEPT').and_return('application/json') - end + let(:http_accept_header) { 'application/json' } it { is_expected.to eq 'application/json' } end end - context 'no configured actions' do - let(:actions) { {} } - subject(:response) { described_class.negotiate(env, actions) } + context 'empty action_params' do + let(:action_params) { {} } + subject(:response) { described_class.build(action_params, http_accept_header) } it 'uses default response' do expect(response.status).to eq 403 @@ -82,12 +67,10 @@ end describe 'redirect_request' do - let(:actions) do + let(:action_params) do { - 'redirect_request' => { - 'location' => location, - 'status_code' => status_code - } + 'location' => location, + 'status_code' => status_code } end @@ -95,7 +78,7 @@ let(:status_code) { '303' } context 'status_code' do - subject(:status) { described_class.negotiate(env, actions).status } + subject(:status) { described_class.build(action_params, http_accept_header).status } it { is_expected.to eq 303 } @@ -107,24 +90,13 @@ end context 'body' do - subject(:body) { described_class.negotiate(env, actions).body } + subject(:body) { described_class.build(action_params, http_accept_header).body } it { is_expected.to eq [] } end context 'headers' do - subject(:headers) { described_class.negotiate(env, actions).headers } - - context 'Content-Type' do - before do - expect(env).to receive(:key?).with('HTTP_ACCEPT').and_return(true) - expect(env).to receive(:[]).with('HTTP_ACCEPT').and_return('application/json') - end - - it 'uses the one from HTTP_ACCEPT header' do - expect(headers['Content-Type']).to eq('application/json') - end - end + subject(:headers) { described_class.build(action_params, http_accept_header).headers } context 'Location' do it 'uses the one from the configuration' do @@ -132,34 +104,17 @@ end end end - - context 'location is empty' do - let(:location) { '' } - - subject(:response) { described_class.negotiate(env, actions) } - - it 'uses default response' do - expect(response.status).to eq 403 - expect(response.body).to eq [Datadog::AppSec::Assets.blocked(format: :html)] - expect(response.headers['Content-Type']).to eq 'text/html' - end - end end end describe '.status' do - subject(:status) { described_class.negotiate(env, {}).status } + subject(:status) { described_class.build({}, http_accept_header).status } it { is_expected.to eq 403 } end describe '.body' do - subject(:body) { described_class.negotiate(env, {}).body } - - before do - expect(env).to receive(:key?).with('HTTP_ACCEPT').and_return(true) - expect(env).to receive(:[]).with('HTTP_ACCEPT').and_return(accept) - end + subject(:body) { described_class.build({}, http_accept_header).body } shared_examples_for 'with custom response body' do |type| before do @@ -176,13 +131,13 @@ end context 'with unsupported Accept headers' do - let(:accept) { 'application/xml' } + let(:http_accept_header) { 'application/xml' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :json)] } end context('with Accept: text/html') do - let(:accept) { 'text/html' } + let(:http_accept_header) { 'text/html' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :html)] } @@ -190,7 +145,7 @@ end context('with Accept: application/json') do - let(:accept) { 'application/json' } + let(:http_accept_header) { 'application/json' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :json)] } @@ -198,7 +153,7 @@ end context('with Accept: text/plain') do - let(:accept) { 'text/plain' } + let(:http_accept_header) { 'text/plain' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :text)] } @@ -207,90 +162,82 @@ end describe ".headers['Content-Type']" do - subject(:content_type) { described_class.negotiate(env, {}).headers['Content-Type'] } - - before do - expect(env).to receive(:key?).with('HTTP_ACCEPT').and_return(respond_to?(:accept)) - - if respond_to?(:accept) - expect(env).to receive(:[]).with('HTTP_ACCEPT').and_return(accept) - else - expect(env).to_not receive(:[]).with('HTTP_ACCEPT') - end - end + subject(:content_type) { described_class.build({}, http_accept_header).headers['Content-Type'] } context('with Accept: text/html') do - let(:accept) { 'text/html' } + let(:http_accept_header) { 'text/html' } - it { is_expected.to eq accept } + it { is_expected.to eq http_accept_header } end context('with Accept: application/json') do - let(:accept) { 'application/json' } + let(:http_accept_header) { 'application/json' } - it { is_expected.to eq accept } + it { is_expected.to eq http_accept_header } end context('with Accept: text/plain') do - let(:accept) { 'text/plain' } + let(:http_accept_header) { 'text/plain' } - it { is_expected.to eq accept } + it { is_expected.to eq http_accept_header } end context('without Accept header') do + let(:http_accept_header) { nil } + it { is_expected.to eq 'application/json' } end context('with Accept: */*') do - let(:accept) { '*/*' } + let(:http_accept_header) { '*/*' } it { is_expected.to eq 'application/json' } end context('with Accept: text/*') do - let(:accept) { 'text/*' } + let(:http_accept_header) { 'text/*' } it { is_expected.to eq 'text/html' } end context('with Accept: application/*') do - let(:accept) { 'application/*' } + let(:http_accept_header) { 'application/*' } it { is_expected.to eq 'application/json' } end context('with unparseable Accept header') do - let(:accept) { 'invalid' } + let(:http_accept_header) { 'invalid' } it { is_expected.to eq 'application/json' } end context('with Accept: text/*;q=0.7, application/*;q=0.8, */*;q=0.9') do - let(:accept) { 'text/*;q=0.7, application/*;q=0.8, */*;q=0.9' } + let(:http_accept_header) { 'text/*;q=0.7, application/*;q=0.8, */*;q=0.9' } it { is_expected.to eq 'application/json' } end context('with unsupported Accept header') do - let(:accept) { 'image/webp' } + let(:http_accept_header) { 'image/webp' } it { is_expected.to eq 'application/json' } end context('with Mozilla Firefox Accept') do - let(:accept) { 'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8' } + let(:http_accept_header) { 'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8' } it { is_expected.to eq 'text/html' } end context('with Google Chrome Accept') do - let(:accept) { 'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7' } # rubocop:disable Layout/LineLength + let(:http_accept_header) { 'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7' } # rubocop:disable Layout/LineLength it { is_expected.to eq 'text/html' } end context('with Apple Safari Accept') do - let(:accept) { 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' } + let(:http_accept_header) { 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' } it { is_expected.to eq 'text/html' } end From f65767bea77e3cf50cab368e61169e53c586f0ce Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Fri, 17 Jan 2025 10:29:30 +0100 Subject: [PATCH 2/6] Change blocking in GraphQL to use ActionHandler --- .../appsec/contrib/graphql/appsec_trace.rb | 8 +---- .../appsec/contrib/graphql/gateway/watcher.rb | 9 +++--- lib/datadog/appsec/response.rb | 14 --------- sig/datadog/appsec/response.rbs | 1 - .../contrib/graphql/integration_test_spec.rb | 30 +++++++------------ 5 files changed, 16 insertions(+), 46 deletions(-) diff --git a/lib/datadog/appsec/contrib/graphql/appsec_trace.rb b/lib/datadog/appsec/contrib/graphql/appsec_trace.rb index 72d74204265..4c9ea738af0 100644 --- a/lib/datadog/appsec/contrib/graphql/appsec_trace.rb +++ b/lib/datadog/appsec/contrib/graphql/appsec_trace.rb @@ -16,16 +16,10 @@ def execute_multiplex(multiplex:) gateway_multiplex = Gateway::Multiplex.new(multiplex) - multiplex_return, multiplex_response = Instrumentation.gateway.push('graphql.multiplex', gateway_multiplex) do + multiplex_return, = Instrumentation.gateway.push('graphql.multiplex', gateway_multiplex) do super end - # Returns an error * the number of queries so that the entire multiplex is blocked - if multiplex_response - blocked_event = multiplex_response.find { |action, _options| action == :block } - multiplex_return = AppSec::Response.graphql_response(gateway_multiplex) if blocked_event - end - multiplex_return end end diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index 3991a3b6669..a414944ec89 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -22,7 +22,6 @@ def watch # This time we don't throw but use next def watch_multiplex(gateway = Instrumentation.gateway) gateway.watch('graphql.multiplex', :appsec) do |stack, gateway_multiplex| - block = false event = nil context = AppSec::Context.active engine = AppSec::Reactive::Engine.new @@ -39,13 +38,15 @@ def watch_multiplex(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event + + result.actions.each do |action_type, action_params| + Datadog::AppSec::ActionHandler.handle(action_type, action_params) + end end - block = GraphQL::Reactive::Multiplex.publish(engine, gateway_multiplex) + GraphQL::Reactive::Multiplex.publish(engine, gateway_multiplex) end - next [nil, [[:block, event]]] if block - stack.call(gateway_multiplex.arguments) end end diff --git a/lib/datadog/appsec/response.rb b/lib/datadog/appsec/response.rb index 33a4c7ccc9b..b01b6a53842 100644 --- a/lib/datadog/appsec/response.rb +++ b/lib/datadog/appsec/response.rb @@ -26,20 +26,6 @@ def build(action_params, http_accept_header) block_response(action_params, http_accept_header) end - def graphql_response(gateway_multiplex) - multiplex_return = [] - gateway_multiplex.queries.each do |query| - # This method is only called in places where GraphQL-Ruby is already required - query_result = ::GraphQL::Query::Result.new( - query: query, - values: JSON.parse(content('application/json')) - ) - multiplex_return << query_result - end - - multiplex_return - end - private def block_response(action_params, http_accept_header) diff --git a/sig/datadog/appsec/response.rbs b/sig/datadog/appsec/response.rbs index 0eb5b47f757..6030ec882ac 100644 --- a/sig/datadog/appsec/response.rbs +++ b/sig/datadog/appsec/response.rbs @@ -10,7 +10,6 @@ module Datadog def to_rack: () -> ::Array[untyped] def self.build: (::Hash[::String, ::String] action_params, ::String http_accept_header) -> Response - def self.graphql_response: (Datadog::AppSec::Contrib::GraphQL::Gateway::Multiplex gateway_multiplex) -> Array[::GraphQL::Query::Result] private diff --git a/spec/datadog/appsec/contrib/graphql/integration_test_spec.rb b/spec/datadog/appsec/contrib/graphql/integration_test_spec.rb index 5209b92355a..2ed9fad4430 100644 --- a/spec/datadog/appsec/contrib/graphql/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/graphql/integration_test_spec.rb @@ -220,10 +220,8 @@ let(:query) { '{ userByName(name: "$testattack") { id } }' } it do - expect(last_response.body).to eq( - { - 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] - }.to_json + expect(JSON.parse(last_response.body)).to eq( + 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] ) expect(spans).to include( an_object_having_attributes( @@ -240,8 +238,7 @@ ) end - # GraphQL errors should have no impact on the HTTP layer - it_behaves_like 'a POST 200 span' + it_behaves_like 'a POST 403 span' it_behaves_like 'a trace with AppSec tags' it_behaves_like 'a trace with AppSec events' end @@ -297,10 +294,8 @@ let(:mutation) { 'mutation { createUser(name: "$testattack") { user { name, id } } }' } it do - expect(last_response.body).to eq( - { - 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] - }.to_json + expect(JSON.parse(last_response.body)).to eq( + 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] ) expect(spans).to include( an_object_having_attributes( @@ -317,7 +312,7 @@ ) end - it_behaves_like 'a POST 200 span' + it_behaves_like 'a POST 403 span' it_behaves_like 'a trace with AppSec tags' it_behaves_like 'a trace with AppSec events' @@ -435,11 +430,8 @@ end it do - expect(last_response.body).to eq( - [ - { 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] }, - { 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] } - ].to_json + expect(JSON.parse(last_response.body)).to eq( + 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] ) expect(spans).to include( an_object_having_attributes( @@ -549,10 +541,8 @@ end it do - expect(last_response.body).to eq( - [ - { 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] } - ].to_json + expect(JSON.parse(last_response.body)).to eq( + 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] ) expect(spans).to include( an_object_having_attributes( From 307cc2a43c7536d006f6299d60281e66a084590c Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Fri, 17 Jan 2025 12:04:29 +0100 Subject: [PATCH 3/6] Switch system-tests workflow to a branch --- .github/workflows/system-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index f7818927b2a..70f663ddfb0 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -11,7 +11,7 @@ on: env: REGISTRY: ghcr.io REPO: ghcr.io/datadog/dd-trace-rb - SYSTEM_TESTS_REF: main # This must always be set to `main` on dd-trace-rb's master branch + SYSTEM_TESTS_REF: appsec-remove-graphql-status-code-exception-for-ruby # This must always be set to `main` on dd-trace-rb's master branch jobs: build-harness: From 93506ec7e4a6e51156f00c10a6ffa2d9918d84ca Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Fri, 17 Jan 2025 16:08:21 +0100 Subject: [PATCH 4/6] Rename AppSec::ActionHandler to AppSec::ActionsHandler Because we need to handle actions according their precedence. --- lib/datadog/appsec/action_handler.rb | 36 ------------------- lib/datadog/appsec/actions_handler.rb | 33 +++++++++++++++++ lib/datadog/appsec/component.rb | 2 +- .../appsec/contrib/graphql/gateway/watcher.rb | 4 +-- .../appsec/contrib/rack/gateway/watcher.rb | 12 ++----- .../appsec/contrib/rails/gateway/watcher.rb | 4 +-- .../appsec/contrib/sinatra/gateway/watcher.rb | 8 ++--- lib/datadog/appsec/monitor/gateway/watcher.rb | 4 +-- ...action_handler.rbs => actions_handler.rbs} | 4 +-- 9 files changed, 44 insertions(+), 63 deletions(-) delete mode 100644 lib/datadog/appsec/action_handler.rb create mode 100644 lib/datadog/appsec/actions_handler.rb rename sig/datadog/appsec/{action_handler.rbs => actions_handler.rbs} (79%) diff --git a/lib/datadog/appsec/action_handler.rb b/lib/datadog/appsec/action_handler.rb deleted file mode 100644 index 6ca3663057d..00000000000 --- a/lib/datadog/appsec/action_handler.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module Datadog - module AppSec - # this module encapsulates functions for handling actions that libddawf returns - module ActionHandler - module_function - - def handle(type, action_params) - case type - when 'block_request' then block_request(action_params) - when 'redirect_request' then redirect_request(action_params) - when 'generate_stack' then generate_stack(action_params) - when 'generate_schema' then generate_schema(action_params) - when 'monitor' then monitor(action_params) - else - Datadog.logger.error "Unknown action type: #{type}" - end - end - - def block_request(action_params) - throw(Datadog::AppSec::Ext::INTERRUPT, action_params) - end - - def redirect_request(action_params) - throw(Datadog::AppSec::Ext::INTERRUPT, action_params) - end - - def generate_stack(_action_params); end - - def generate_schema(_action_params); end - - def monitor(_action_params); end - end - end -end diff --git a/lib/datadog/appsec/actions_handler.rb b/lib/datadog/appsec/actions_handler.rb new file mode 100644 index 00000000000..e970dc73461 --- /dev/null +++ b/lib/datadog/appsec/actions_handler.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + # this module encapsulates functions for handling actions that libddawf returns + module ActionsHandler + module_function + + def handle(actions_hash) + # handle actions according their precedence + # stack and schema generation should be done before we throw an interrupt signal + generate_stack(actions_hash['generate_stack']) if actions_hash.key?('generate_stack') + generate_schema(actions_hash['generate_schema']) if actions_hash.key?('generate_schema') + redirect_request(actions_hash['redirect_request']) if actions_hash.key?('redirect_request') + block_request(actions_hash['block_request']) if actions_hash.key?('block_request') + end + + def block_request(action_params) + throw(Datadog::AppSec::Ext::INTERRUPT, action_params) + end + + def redirect_request(action_params) + throw(Datadog::AppSec::Ext::INTERRUPT, action_params) + end + + def generate_stack(_action_params); end + + def generate_schema(_action_params); end + + def monitor(_action_params); end + end + end +end diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index 097c32e00e7..7951989e0fa 100644 --- a/lib/datadog/appsec/component.rb +++ b/lib/datadog/appsec/component.rb @@ -3,7 +3,7 @@ require_relative 'processor' require_relative 'processor/rule_merger' require_relative 'processor/rule_loader' -require_relative 'action_handler' +require_relative 'actions_handler' module Datadog module AppSec diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index a414944ec89..d62a66fe966 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -39,9 +39,7 @@ def watch_multiplex(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end GraphQL::Reactive::Multiplex.publish(engine, gateway_multiplex) diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 4f68d746755..d5cec4b96b0 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -45,9 +45,7 @@ def watch_request(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end end @@ -79,9 +77,7 @@ def watch_response(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end end @@ -113,9 +109,7 @@ def watch_request_body(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end end diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index 44a7a3a7d2a..cd6a67f4253 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -41,9 +41,7 @@ def watch_request_action(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index f0e99a31ada..10f6eaf3bd5 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -43,9 +43,7 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end end @@ -77,9 +75,7 @@ def watch_request_routed(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end end diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 64a9c31c532..764ec88095a 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -39,9 +39,7 @@ def watch_user_id(gateway = Instrumentation.gateway) Datadog::AppSec::Event.tag_and_keep!(context, result) context.events << event - result.actions.each do |action_type, action_params| - Datadog::AppSec::ActionHandler.handle(action_type, action_params) - end + Datadog::AppSec::ActionsHandler.handle(result.actions) end end diff --git a/sig/datadog/appsec/action_handler.rbs b/sig/datadog/appsec/actions_handler.rbs similarity index 79% rename from sig/datadog/appsec/action_handler.rbs rename to sig/datadog/appsec/actions_handler.rbs index 86fab71f40d..0e7535f43ad 100644 --- a/sig/datadog/appsec/action_handler.rbs +++ b/sig/datadog/appsec/actions_handler.rbs @@ -1,7 +1,7 @@ module Datadog module AppSec - module ActionHandler - def handle: (::String type, ::Hash[::String, ::String] action_params) -> void + module ActionsHandler + def handle: (::Hash[::String, ::Hash[::String, ::String]] actions_hash) -> void def block_request: (::Hash[::String, ::String] action_params) -> void From 06f2060bd34c2153e6485dd70cc590ca6571c909 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Fri, 17 Jan 2025 16:46:24 +0100 Subject: [PATCH 5/6] Add tests for AppSec::ActionsHandler We want to test that ActionsHandler handles actions correctly according their precedence. --- spec/datadog/appsec/actions_handler_spec.rb | 107 ++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 spec/datadog/appsec/actions_handler_spec.rb diff --git a/spec/datadog/appsec/actions_handler_spec.rb b/spec/datadog/appsec/actions_handler_spec.rb new file mode 100644 index 00000000000..71e8923fe51 --- /dev/null +++ b/spec/datadog/appsec/actions_handler_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'datadog/appsec/spec_helper' + +RSpec.describe Datadog::AppSec::ActionsHandler do + describe '.handle' do + let(:generate_stack_action) do + { 'generate_stack' => { 'stack_id' => 'foo' } } + end + + let(:generate_schema_action) do + { 'generate_schema' => {} } + end + + let(:redirect_request_action) do + { 'redirect_request' => { 'status_code' => '303', 'location' => 'http://example.com' } } + end + + let(:block_request_action) do + { 'block_request' => { 'status_code' => '403', 'type' => 'auto' } } + end + + it 'calls generate_stack with action parameters' do + expect(described_class).to( + receive(:generate_stack).with(generate_stack_action['generate_stack']).and_call_original + ) + + described_class.handle(generate_stack_action) + end + + it 'calls generate_schema with action parameters' do + expect(described_class).to( + receive(:generate_schema).with(generate_schema_action['generate_schema']).and_call_original + ) + + described_class.handle('generate_schema' => generate_schema_action['generate_schema']) + end + + it 'calls redirect_request with action parameters' do + expect(described_class).to( + receive(:redirect_request).with(redirect_request_action['redirect_request']).and_call_original + ) + + catch(Datadog::AppSec::Ext::INTERRUPT) do + described_class.handle(redirect_request_action) + end + end + + it 'calls block_request with action parameters' do + expect(described_class).to( + receive(:block_request).with(block_request_action['block_request']).and_call_original + ) + + catch(Datadog::AppSec::Ext::INTERRUPT) do + described_class.handle(block_request_action) + end + end + + it 'calls redirect_request only when both block_request and redirect_request are present' do + expect(described_class).to( + receive(:redirect_request).with(redirect_request_action['redirect_request']).and_call_original + ) + expect(described_class).not_to receive(:block_request) + + catch(Datadog::AppSec::Ext::INTERRUPT) do + described_class.handle(**block_request_action, **redirect_request_action) + end + end + + it 'calls both generate_stack and generate_schema when both are present' do + expect(described_class).to( + receive(:generate_stack).with(generate_stack_action['generate_stack']).and_call_original + ) + expect(described_class).to( + receive(:generate_schema).with(generate_schema_action['generate_schema']).and_call_original + ) + + described_class.handle(**generate_stack_action, **generate_schema_action) + end + + it 'calls both generate_stack and block_request when both are present' do + expect(described_class).to( + receive(:generate_stack).with(generate_stack_action['generate_stack']).and_call_original + ) + expect(described_class).to( + receive(:block_request).with(block_request_action['block_request']).and_call_original + ) + + catch(Datadog::AppSec::Ext::INTERRUPT) do + described_class.handle(**generate_stack_action, **block_request_action) + end + end + + it 'calls both generate_stack and redirect_request when both are present' do + expect(described_class).to( + receive(:generate_stack).with(generate_stack_action['generate_stack']).and_call_original + ) + expect(described_class).to( + receive(:redirect_request).with(redirect_request_action['redirect_request']).and_call_original + ) + + catch(Datadog::AppSec::Ext::INTERRUPT) do + described_class.handle(**generate_stack_action, **redirect_request_action) + end + end + end +end From 4d473606416d5b843524a69eac2be8a2b4377739 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Fri, 17 Jan 2025 16:51:19 +0100 Subject: [PATCH 6/6] Remove double-splat from ActionsHandler spec These examples have to be rewritten with Hash#merge, since older versions of Ruby do not support double-splat operator. --- spec/datadog/appsec/actions_handler_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/datadog/appsec/actions_handler_spec.rb b/spec/datadog/appsec/actions_handler_spec.rb index 71e8923fe51..3383c3db6d7 100644 --- a/spec/datadog/appsec/actions_handler_spec.rb +++ b/spec/datadog/appsec/actions_handler_spec.rb @@ -63,7 +63,7 @@ expect(described_class).not_to receive(:block_request) catch(Datadog::AppSec::Ext::INTERRUPT) do - described_class.handle(**block_request_action, **redirect_request_action) + described_class.handle(block_request_action.merge(redirect_request_action)) end end @@ -75,7 +75,7 @@ receive(:generate_schema).with(generate_schema_action['generate_schema']).and_call_original ) - described_class.handle(**generate_stack_action, **generate_schema_action) + described_class.handle(generate_stack_action.merge(generate_schema_action)) end it 'calls both generate_stack and block_request when both are present' do @@ -87,7 +87,7 @@ ) catch(Datadog::AppSec::Ext::INTERRUPT) do - described_class.handle(**generate_stack_action, **block_request_action) + described_class.handle(generate_stack_action.merge(block_request_action)) end end @@ -100,7 +100,7 @@ ) catch(Datadog::AppSec::Ext::INTERRUPT) do - described_class.handle(**generate_stack_action, **redirect_request_action) + described_class.handle(generate_stack_action.merge(redirect_request_action)) end end end