From 997e30d583d6214349d0d8aebc4b6ca200af2904 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Thu, 14 Nov 2024 10:31:36 +0100 Subject: [PATCH 1/9] Add ActiveRecord instrumentation to detect SQLi in AppSec --- lib/datadog/appsec.rb | 1 + .../contrib/active_record/integration.rb | 41 +++++++++++++++++ .../active_record/mysql2_adapter_patch.rb | 39 ++++++++++++++++ .../appsec/contrib/active_record/patcher.rb | 45 +++++++++++++++++++ .../active_record/postgresql_adapter_patch.rb | 39 ++++++++++++++++ .../active_record/sqlite3_adapter_patch.rb | 40 +++++++++++++++++ 6 files changed, 205 insertions(+) create mode 100644 lib/datadog/appsec/contrib/active_record/integration.rb create mode 100644 lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb create mode 100644 lib/datadog/appsec/contrib/active_record/patcher.rb create mode 100644 lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb create mode 100644 lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index 940bbc4b4b6..7341af4c790 100644 --- a/lib/datadog/appsec.rb +++ b/lib/datadog/appsec.rb @@ -53,6 +53,7 @@ def components end # Integrations +require_relative 'appsec/contrib/active_record/integration' require_relative 'appsec/contrib/rack/integration' require_relative 'appsec/contrib/sinatra/integration' require_relative 'appsec/contrib/rails/integration' diff --git a/lib/datadog/appsec/contrib/active_record/integration.rb b/lib/datadog/appsec/contrib/active_record/integration.rb new file mode 100644 index 00000000000..022067d4e19 --- /dev/null +++ b/lib/datadog/appsec/contrib/active_record/integration.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require_relative '../integration' +require_relative 'patcher' + +module Datadog + module AppSec + module Contrib + module ActiveRecord + # Description of ActiveRecord integration + class Integration + include Datadog::AppSec::Contrib::Integration + + MINIMUM_VERSION = Gem::Version.new('4') + + register_as :active_record, auto_patch: false + + def self.version + Gem.loaded_specs['activerecord'] && Gem.loaded_specs['activerecord'].version + end + + def self.loaded? + !defined?(::ActiveRecord).nil? + end + + def self.compatible? + super && version >= MINIMUM_VERSION + end + + def self.auto_instrument? + true + end + + def patcher + Patcher + end + end + end + end + end +end diff --git a/lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb b/lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb new file mode 100644 index 00000000000..cb889634aaa --- /dev/null +++ b/lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + module Contrib + module ActiveRecord + # AppSec module that will be prepended to ActiveRecord adapter + module Mysql2AdapterPatch + def internal_exec_query(sql, *args, **rest) + scope = AppSec.active_scope + return super unless scope + + ephemeral_data = { + 'server.db.statement' => sql, + 'server.db.system' => 'mysql' + } + + result = scope.processor_context.run({}, ephemeral_data, Datadog.configuration.appsec.waf_timeout) + + if result.status == :match + Datadog::AppSec::Event.tag_and_keep!(scope, result) + + event = { + waf_result: result, + trace: scope.trace, + span: scope.service_entry_span, + sql: sql, + actions: result.actions + } + scope.processor_context.events << event + end + + super + end + end + end + end + end +end diff --git a/lib/datadog/appsec/contrib/active_record/patcher.rb b/lib/datadog/appsec/contrib/active_record/patcher.rb new file mode 100644 index 00000000000..24806087519 --- /dev/null +++ b/lib/datadog/appsec/contrib/active_record/patcher.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require_relative '../patcher' +require_relative 'sqlite3_adapter_patch' +require_relative 'postgresql_adapter_patch' +require_relative 'mysql2_adapter_patch' + +module Datadog + module AppSec + module Contrib + module ActiveRecord + # AppSec patcher module for ActiveRecord + module Patcher + include Datadog::AppSec::Contrib::Patcher + + module_function + + def patched? + Patcher.instance_variable_get(:@patched) + end + + def target_version + Integration.version + end + + def patch + ActiveSupport.on_load :active_record do + if defined? ::ActiveRecord::ConnectionAdapters::SQLite3Adapter + ::ActiveRecord::ConnectionAdapters::SQLite3Adapter.prepend(SQLite3AdapterPatch) + end + + if defined? ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter + ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(PostgreSQLAdapterPatch) + end + + if defined? ::ActiveRecord::ConnectionAdapters::Mysql2Adapter + ::ActiveRecord::ConnectionAdapters::Mysql2Adapter.prepend(Mysql2AdapterPatch) + end + end + end + end + end + end + end +end diff --git a/lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb b/lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb new file mode 100644 index 00000000000..98f0e26f808 --- /dev/null +++ b/lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + module Contrib + module ActiveRecord + # AppSec module that will be prepended to ActiveRecord adapter + module PostgreSQLAdapterPatch + def internal_exec_query(sql, *args, **rest) + scope = AppSec.active_scope + return super unless scope + + ephemeral_data = { + 'server.db.statement' => sql, + 'server.db.system' => 'postgresql' + } + + result = scope.processor_context.run({}, ephemeral_data, Datadog.configuration.appsec.waf_timeout) + + if result.status == :match + Datadog::AppSec::Event.tag_and_keep!(scope, result) + + event = { + waf_result: result, + trace: scope.trace, + span: scope.service_entry_span, + actions: result.actions, + sql: sql + } + scope.processor_context.events << event + end + + super + end + end + end + end + end +end diff --git a/lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb b/lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb new file mode 100644 index 00000000000..e8c0bfa6e8d --- /dev/null +++ b/lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + module Contrib + module ActiveRecord + # AppSec module that will be prepended to ActiveRecord adapter + module SQLite3AdapterPatch + def internal_exec_query(sql, *args, **rest) + scope = AppSec.active_scope + return super unless scope + + ephemeral_data = { + 'server.db.statement' => sql, + 'server.db.system' => 'sqlite' + } + + waf_timeout = Datadog.configuration.appsec.waf_timeout + result = scope.processor_context.run({}, ephemeral_data, waf_timeout) + + if result.status == :match + Datadog::AppSec::Event.tag_and_keep!(scope, result) + + event = { + waf_result: result, + trace: scope.trace, + span: scope.service_entry_span, + sql: sql, + actions: result.actions + } + scope.processor_context.events << event + end + + super + end + end + end + end + end +end From ff6863f2254f7e3edd98b6d997393d33cec107bd Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Fri, 22 Nov 2024 12:35:45 +0100 Subject: [PATCH 2/9] Add patching for older rails versions --- .../contrib/active_record/instrumentation.rb | 67 +++++++++++++++++++ .../active_record/mysql2_adapter_patch.rb | 39 ----------- .../appsec/contrib/active_record/patcher.rb | 20 ++++-- .../active_record/postgresql_adapter_patch.rb | 39 ----------- .../active_record/sqlite3_adapter_patch.rb | 40 ----------- 5 files changed, 81 insertions(+), 124 deletions(-) create mode 100644 lib/datadog/appsec/contrib/active_record/instrumentation.rb delete mode 100644 lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb delete mode 100644 lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb delete mode 100644 lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb diff --git a/lib/datadog/appsec/contrib/active_record/instrumentation.rb b/lib/datadog/appsec/contrib/active_record/instrumentation.rb new file mode 100644 index 00000000000..17ef8ee5d31 --- /dev/null +++ b/lib/datadog/appsec/contrib/active_record/instrumentation.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + module Contrib + module ActiveRecord + # AppSec module that will be prepended to ActiveRecord adapter + module Instrumentation + module_function + + def detect_sqli(sql, adapter_name) + scope = AppSec.active_scope + return unless scope + + ephemeral_data = { + 'server.db.statement' => sql, + 'server.db.system' => adapter_name.downcase.gsub(/\d{1}\z/, '') + } + + waf_timeout = Datadog.configuration.appsec.waf_timeout + result = scope.processor_context.run({}, ephemeral_data, waf_timeout) + + if result.status == :match + Datadog::AppSec::Event.tag_and_keep!(scope, result) + + event = { + waf_result: result, + trace: scope.trace, + span: scope.service_entry_span, + sql: sql, + actions: result.actions + } + scope.processor_context.events << event + end + end + + # patch for all adapters in ActiveRecord >= 7.1 + module InternalExecQueryAdapterPatch + def internal_exec_query(sql, *args, **rest) + Instrumentation.detect_sqli(sql, adapter_name) + + super + end + end + + # patch for postgres adapter in ActiveRecord < 7.1 + module ExecuteAndClearAdapterPatch + def execute_and_clear(sql, *args, **rest) + Instrumentation.detect_sqli(sql, adapter_name) + + super + end + end + + # patch for mysql2 and sqlite3 adapters in ActiveRecord < 7.1 + module ExecQueryAdapterPatch + def exec_query(sql, *args, **rest) + Instrumentation.detect_sqli(sql, adapter_name) + + super + end + end + end + end + end + end +end diff --git a/lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb b/lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb deleted file mode 100644 index cb889634aaa..00000000000 --- a/lib/datadog/appsec/contrib/active_record/mysql2_adapter_patch.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -module Datadog - module AppSec - module Contrib - module ActiveRecord - # AppSec module that will be prepended to ActiveRecord adapter - module Mysql2AdapterPatch - def internal_exec_query(sql, *args, **rest) - scope = AppSec.active_scope - return super unless scope - - ephemeral_data = { - 'server.db.statement' => sql, - 'server.db.system' => 'mysql' - } - - result = scope.processor_context.run({}, ephemeral_data, Datadog.configuration.appsec.waf_timeout) - - if result.status == :match - Datadog::AppSec::Event.tag_and_keep!(scope, result) - - event = { - waf_result: result, - trace: scope.trace, - span: scope.service_entry_span, - sql: sql, - actions: result.actions - } - scope.processor_context.events << event - end - - super - end - end - end - end - end -end diff --git a/lib/datadog/appsec/contrib/active_record/patcher.rb b/lib/datadog/appsec/contrib/active_record/patcher.rb index 24806087519..61acdab9bce 100644 --- a/lib/datadog/appsec/contrib/active_record/patcher.rb +++ b/lib/datadog/appsec/contrib/active_record/patcher.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true require_relative '../patcher' -require_relative 'sqlite3_adapter_patch' -require_relative 'postgresql_adapter_patch' -require_relative 'mysql2_adapter_patch' +require_relative 'instrumentation' module Datadog module AppSec @@ -26,18 +24,28 @@ def target_version def patch ActiveSupport.on_load :active_record do if defined? ::ActiveRecord::ConnectionAdapters::SQLite3Adapter - ::ActiveRecord::ConnectionAdapters::SQLite3Adapter.prepend(SQLite3AdapterPatch) + ::ActiveRecord::ConnectionAdapters::SQLite3Adapter.prepend(Patcher.prepended_class_name(:sqlite3)) end if defined? ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter - ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(PostgreSQLAdapterPatch) + ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(Patcher.prepended_class_name(:postgresql)) end if defined? ::ActiveRecord::ConnectionAdapters::Mysql2Adapter - ::ActiveRecord::ConnectionAdapters::Mysql2Adapter.prepend(Mysql2AdapterPatch) + ::ActiveRecord::ConnectionAdapters::Mysql2Adapter.prepend(Patcher.prepended_class_name(:mysql2)) end end end + + def prepended_class_name(adapter_name) + if ::ActiveRecord.gem_version >= Gem::Version.new('7.1') + Instrumentation::InternalExecQueryAdapterPatch + elsif adapter_name == :postgresql + Instrumentation::ExecuteAndClearAdapterPatch + else + Instrumentation::ExecQueryAdapterPatch + end + end end end end diff --git a/lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb b/lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb deleted file mode 100644 index 98f0e26f808..00000000000 --- a/lib/datadog/appsec/contrib/active_record/postgresql_adapter_patch.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -module Datadog - module AppSec - module Contrib - module ActiveRecord - # AppSec module that will be prepended to ActiveRecord adapter - module PostgreSQLAdapterPatch - def internal_exec_query(sql, *args, **rest) - scope = AppSec.active_scope - return super unless scope - - ephemeral_data = { - 'server.db.statement' => sql, - 'server.db.system' => 'postgresql' - } - - result = scope.processor_context.run({}, ephemeral_data, Datadog.configuration.appsec.waf_timeout) - - if result.status == :match - Datadog::AppSec::Event.tag_and_keep!(scope, result) - - event = { - waf_result: result, - trace: scope.trace, - span: scope.service_entry_span, - actions: result.actions, - sql: sql - } - scope.processor_context.events << event - end - - super - end - end - end - end - end -end diff --git a/lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb b/lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb deleted file mode 100644 index e8c0bfa6e8d..00000000000 --- a/lib/datadog/appsec/contrib/active_record/sqlite3_adapter_patch.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -module Datadog - module AppSec - module Contrib - module ActiveRecord - # AppSec module that will be prepended to ActiveRecord adapter - module SQLite3AdapterPatch - def internal_exec_query(sql, *args, **rest) - scope = AppSec.active_scope - return super unless scope - - ephemeral_data = { - 'server.db.statement' => sql, - 'server.db.system' => 'sqlite' - } - - waf_timeout = Datadog.configuration.appsec.waf_timeout - result = scope.processor_context.run({}, ephemeral_data, waf_timeout) - - if result.status == :match - Datadog::AppSec::Event.tag_and_keep!(scope, result) - - event = { - waf_result: result, - trace: scope.trace, - span: scope.service_entry_span, - sql: sql, - actions: result.actions - } - scope.processor_context.events << event - end - - super - end - end - end - end - end -end From 49eb123b228a91ff4f3c869a39c1fede475e7d09 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Mon, 25 Nov 2024 13:48:50 +0100 Subject: [PATCH 3/9] Add specs for AppSec ActiveRecord SQLi detection --- Matrixfile | 3 + Rakefile | 3 +- .../active_record/multi_adapter_spec.rb | 150 ++++++++++++++++++ .../contrib/active_record/patcher_spec.rb | 44 +++++ 4 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb create mode 100644 spec/datadog/appsec/contrib/active_record/patcher_spec.rb diff --git a/Matrixfile b/Matrixfile index 304a1a3a48e..2fa275e5f14 100644 --- a/Matrixfile +++ b/Matrixfile @@ -258,6 +258,9 @@ 'redis-4' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', 'redis-5' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby' }, + 'appsec:active_record' => { + 'relational_db' => '❌ 2.5 / ❌ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', + }, 'appsec:rack' => { 'rack-latest' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', 'rack-3' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', diff --git a/Rakefile b/Rakefile index 41426800e51..5ddc8e14b16 100644 --- a/Rakefile +++ b/Rakefile @@ -267,7 +267,7 @@ namespace :spec do end namespace :appsec do - task all: [:main, :rack, :rails, :sinatra, :devise, :graphql] + task all: [:main, :active_record, :rack, :rails, :sinatra, :devise, :graphql] # Datadog AppSec main specs desc '' # "Explicitly hiding from `rake -T`" @@ -280,6 +280,7 @@ namespace :spec do # Datadog AppSec integrations [ + :active_record, :rack, :sinatra, :rails, diff --git a/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb b/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb new file mode 100644 index 00000000000..7684087f8be --- /dev/null +++ b/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb @@ -0,0 +1,150 @@ +require 'datadog/appsec/spec_helper' +require 'active_record' + +require 'spec/datadog/tracing/contrib/rails/support/deprecation' + +require 'mysql2' +require 'sqlite3' +require 'pg' + +RSpec.describe 'AppSec ActiveRecord integration' do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) } + let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset, telemetry: telemetry) } + let(:context) { processor.new_context } + + let(:span) { Datadog::Tracing::SpanOperation.new('root') } + let(:trace) { Datadog::Tracing::TraceOperation.new } + + let!(:user_class) do + stub_const('User', Class.new(ActiveRecord::Base)).tap do |klass| + klass.establish_connection(db_config) + + klass.connection.create_table 'users', force: :cascade do |t| + t.string :name, null: false + t.string :email, null: false + t.timestamps + end + + # prevent internal sql requests from showing up + klass.count + end + end + + before do + Datadog.configure do |c| + c.appsec.enabled = true + c.appsec.instrument :active_record + end + + Datadog::AppSec::Scope.activate_scope(trace, span, processor) + + raise_on_rails_deprecation! + end + + after do + Datadog.configuration.reset! + + Datadog::AppSec::Scope.deactivate_scope + processor.finalize + end + + shared_examples 'calls_waf_with_correct_arguments' do + it 'calls waf with correct arguments' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).with( + {}, + { + 'server.db.statement' => expected_db_statement, + 'server.db.system' => expected_db_system + }, + Datadog.configuration.appsec.waf_timeout + ).and_call_original + ) + + active_record_scope.to_a + end + end + + context 'mysql2 adapter' do + let(:db_config) do + { + adapter: 'mysql2', + database: ENV.fetch('TEST_MYSQL_DB', 'mysql'), + host: ENV.fetch('TEST_MYSQL_HOST', '127.0.0.1'), + password: ENV.fetch('TEST_MYSQL_ROOT_PASSWORD', 'root'), + port: ENV.fetch('TEST_MYSQL_PORT', '3306') + } + end + + let(:expected_db_system) { 'mysql' } + + context 'when using .where' do + let(:active_record_scope) { User.where(name: 'Bob') } + let(:expected_db_statement) { "SELECT `users`.* FROM `users` WHERE `users`.`name` = 'Bob'" } + + include_examples 'calls_waf_with_correct_arguments' + end + + context 'when using .find_by_sql' do + let(:active_record_scope) { User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'") } + let(:expected_db_statement) { "SELECT * FROM users WHERE name = 'Bob'" } + + include_examples 'calls_waf_with_correct_arguments' + end + end + + context 'postgres adapter' do + let(:db_config) do + { + adapter: 'postgresql', + database: ENV.fetch('TEST_POSTGRES_DB', 'postgres'), + host: ENV.fetch('TEST_POSTGRES_HOST', '127.0.0.1'), + port: ENV.fetch('TEST_POSTGRES_PORT', 5432), + username: ENV.fetch('TEST_POSTGRES_USER', 'postgres'), + password: ENV.fetch('TEST_POSTGRES_PASSWORD', 'postgres') + } + end + + let(:expected_db_system) { 'postgresql' } + + context 'when using .where' do + let(:active_record_scope) { User.where(name: 'Bob') } + let(:expected_db_statement) { 'SELECT "users".* FROM "users" WHERE "users"."name" = $1' } + + include_examples 'calls_waf_with_correct_arguments' + end + + context 'when using .find_by_sql' do + let(:active_record_scope) { User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'") } + let(:expected_db_statement) { "SELECT * FROM users WHERE name = 'Bob'" } + + include_examples 'calls_waf_with_correct_arguments' + end + end + + context 'sqlite3 adapter' do + let(:db_config) do + { + adapter: 'sqlite3', + database: ':memory:' + } + end + + let(:expected_db_system) { 'sqlite' } + + context 'when using .where' do + let(:active_record_scope) { User.where(name: 'Bob') } + let(:expected_db_statement) { 'SELECT "users".* FROM "users" WHERE "users"."name" = ?' } + + include_examples 'calls_waf_with_correct_arguments' + end + + context 'when using .find_by_sql' do + let(:active_record_scope) { User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'") } + let(:expected_db_statement) { "SELECT * FROM users WHERE name = 'Bob'" } + + include_examples 'calls_waf_with_correct_arguments' + end + end +end diff --git a/spec/datadog/appsec/contrib/active_record/patcher_spec.rb b/spec/datadog/appsec/contrib/active_record/patcher_spec.rb new file mode 100644 index 00000000000..976d3a0eca6 --- /dev/null +++ b/spec/datadog/appsec/contrib/active_record/patcher_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'datadog/appsec/spec_helper' +require 'datadog/appsec/contrib/active_record/patcher' + +RSpec.describe Datadog::AppSec::Contrib::ActiveRecord::Patcher do + describe '#prepended_class_name' do + before do + stub_const('::ActiveRecord', Struct.new(:gem_version).new(Gem::Version.new(active_record_version))) + end + + context 'when ActiveRecord version is 7.1 or higher' do + let(:active_record_version) { Gem::Version.new('7.1') } + + it 'returns Instrumentation::InternalExecQueryAdapterPatch' do + expect(described_class.prepended_class_name(:postgresql)).to eq( + Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::InternalExecQueryAdapterPatch + ) + end + end + + context 'when ActiveRecord version is lower than 7.1' do + let(:active_record_version) { Gem::Version.new('7.0') } + + context 'for postgresql adapter' do + it 'returns Instrumentation::ExecuteAndClearAdapterPatch' do + expect(described_class.prepended_class_name(:postgresql)).to eq( + Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::ExecuteAndClearAdapterPatch + ) + end + end + + %i[mysql2 sqlite3].each do |adapter_name| + context "for #{adapter_name} adapter" do + it 'returns Instrumentation::ExecQueryAdapterPatch' do + expect(described_class.prepended_class_name(adapter_name)).to eq( + Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::ExecQueryAdapterPatch + ) + end + end + end + end + end +end From 1f0a7a1d7ea2da92af8e5ca5043889cf2b98d450 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Tue, 26 Nov 2024 16:53:05 +0100 Subject: [PATCH 4/9] Add RBS types for AppSec ActiveRecord instrumentation --- .../contrib/active_record/instrumentation.rbs | 23 +++++++++++++++++++ .../contrib/active_record/integration.rbs | 23 +++++++++++++++++++ .../appsec/contrib/active_record/patcher.rbs | 19 +++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 sig/datadog/appsec/contrib/active_record/instrumentation.rbs create mode 100644 sig/datadog/appsec/contrib/active_record/integration.rbs create mode 100644 sig/datadog/appsec/contrib/active_record/patcher.rbs diff --git a/sig/datadog/appsec/contrib/active_record/instrumentation.rbs b/sig/datadog/appsec/contrib/active_record/instrumentation.rbs new file mode 100644 index 00000000000..b81b4138642 --- /dev/null +++ b/sig/datadog/appsec/contrib/active_record/instrumentation.rbs @@ -0,0 +1,23 @@ +module Datadog + module AppSec + module Contrib + module ActiveRecord + module Instrumentation + def self?.detect_sqli: (String sql, String adapter_name) -> void + + module InternalExecQueryAdapterPatch + def internal_exec_query: (String sql, *untyped args, **untyped rest) -> untyped + end + + module ExecuteAndClearAdapterPatch + def execute_and_clear: (String sql, *untyped args, **untyped rest) -> untyped + end + + module ExecQueryAdapterPatch + def exec_query: (String sql, *untyped args, **untyped rest) -> untyped + end + end + end + end + end +end diff --git a/sig/datadog/appsec/contrib/active_record/integration.rbs b/sig/datadog/appsec/contrib/active_record/integration.rbs new file mode 100644 index 00000000000..781f186efc1 --- /dev/null +++ b/sig/datadog/appsec/contrib/active_record/integration.rbs @@ -0,0 +1,23 @@ +module Datadog + module AppSec + module Contrib + module ActiveRecord + class Integration + include Datadog::AppSec::Contrib::Integration + + MINIMUM_VERSION: Gem::Version + + def self.version: () -> Gem::Version? + + def self.loaded?: () -> bool + + def self.compatible?: () -> bool + + def self.auto_instrument?: () -> true + + def patcher: () -> class + end + end + end + end +end diff --git a/sig/datadog/appsec/contrib/active_record/patcher.rbs b/sig/datadog/appsec/contrib/active_record/patcher.rbs new file mode 100644 index 00000000000..609b75bf6fb --- /dev/null +++ b/sig/datadog/appsec/contrib/active_record/patcher.rbs @@ -0,0 +1,19 @@ +module Datadog + module AppSec + module Contrib + module ActiveRecord + module Patcher + include Datadog::AppSec::Contrib::Patcher + + def self?.patched?: () -> bool + + def self?.target_version: () -> Gem::Version? + + def self?.patch: () -> void + + def self?.prepended_class_name: (Symbol adapter_name) -> class + end + end + end + end +end From 2edc1a0473a35b6347bdae88910dd1702e992e26 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Tue, 26 Nov 2024 17:48:25 +0100 Subject: [PATCH 5/9] Fix AppSec ActiveRecord integration specs for JRuby --- .../contrib/active_record/instrumentation.rb | 1 + .../appsec/contrib/active_record/patcher.rb | 2 +- .../active_record/multi_adapter_spec.rb | 19 ++++++++-- .../contrib/active_record/patcher_spec.rb | 38 ++++++++++++++++--- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/lib/datadog/appsec/contrib/active_record/instrumentation.rb b/lib/datadog/appsec/contrib/active_record/instrumentation.rb index 17ef8ee5d31..e8103410e35 100644 --- a/lib/datadog/appsec/contrib/active_record/instrumentation.rb +++ b/lib/datadog/appsec/contrib/active_record/instrumentation.rb @@ -53,6 +53,7 @@ def execute_and_clear(sql, *args, **rest) end # patch for mysql2 and sqlite3 adapters in ActiveRecord < 7.1 + # this patch is also used when using JDBC adapter module ExecQueryAdapterPatch def exec_query(sql, *args, **rest) Instrumentation.detect_sqli(sql, adapter_name) diff --git a/lib/datadog/appsec/contrib/active_record/patcher.rb b/lib/datadog/appsec/contrib/active_record/patcher.rb index 61acdab9bce..b6e65b97d51 100644 --- a/lib/datadog/appsec/contrib/active_record/patcher.rb +++ b/lib/datadog/appsec/contrib/active_record/patcher.rb @@ -40,7 +40,7 @@ def patch def prepended_class_name(adapter_name) if ::ActiveRecord.gem_version >= Gem::Version.new('7.1') Instrumentation::InternalExecQueryAdapterPatch - elsif adapter_name == :postgresql + elsif adapter_name == :postgresql && !defined?(::ActiveRecord::ConnectionAdapters::JdbcAdapter) Instrumentation::ExecuteAndClearAdapterPatch else Instrumentation::ExecQueryAdapterPatch diff --git a/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb b/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb index 7684087f8be..7ce35364975 100644 --- a/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb +++ b/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb @@ -3,9 +3,13 @@ require 'spec/datadog/tracing/contrib/rails/support/deprecation' -require 'mysql2' -require 'sqlite3' -require 'pg' +if PlatformHelpers.jruby? + require 'activerecord-jdbc-adapter' +else + require 'mysql2' + require 'sqlite3' + require 'pg' +end RSpec.describe 'AppSec ActiveRecord integration' do let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } @@ -28,6 +32,7 @@ # prevent internal sql requests from showing up klass.count + klass.first end end @@ -110,7 +115,13 @@ context 'when using .where' do let(:active_record_scope) { User.where(name: 'Bob') } - let(:expected_db_statement) { 'SELECT "users".* FROM "users" WHERE "users"."name" = $1' } + let(:expected_db_statement) do + if PlatformHelpers.jruby? + 'SELECT "users".* FROM "users" WHERE "users"."name" = ?' + else + 'SELECT "users".* FROM "users" WHERE "users"."name" = $1' + end + end include_examples 'calls_waf_with_correct_arguments' end diff --git a/spec/datadog/appsec/contrib/active_record/patcher_spec.rb b/spec/datadog/appsec/contrib/active_record/patcher_spec.rb index 976d3a0eca6..468b9631a15 100644 --- a/spec/datadog/appsec/contrib/active_record/patcher_spec.rb +++ b/spec/datadog/appsec/contrib/active_record/patcher_spec.rb @@ -5,12 +5,17 @@ RSpec.describe Datadog::AppSec::Contrib::ActiveRecord::Patcher do describe '#prepended_class_name' do - before do - stub_const('::ActiveRecord', Struct.new(:gem_version).new(Gem::Version.new(active_record_version))) - end - context 'when ActiveRecord version is 7.1 or higher' do - let(:active_record_version) { Gem::Version.new('7.1') } + before do + stub_const( + '::ActiveRecord', + Module.new do + module_function def gem_version + Gem::Version.new('7.1') + end + end + ) + end it 'returns Instrumentation::InternalExecQueryAdapterPatch' do expect(described_class.prepended_class_name(:postgresql)).to eq( @@ -20,9 +25,30 @@ end context 'when ActiveRecord version is lower than 7.1' do - let(:active_record_version) { Gem::Version.new('7.0') } + before do + stub_const( + '::ActiveRecord', + Module.new do + module_function def gem_version + Gem::Version.new('7.0') + end + end + ) + end context 'for postgresql adapter' do + context 'when ActiveRecord::ConnectionAdapters::JdbcAdapter is defined' do + before do + stub_const('::ActiveRecord::ConnectionAdapters::JdbcAdapter', Class.new) + end + + it 'returns Instrumentation::ExecQueryAdapterPatch' do + expect(described_class.prepended_class_name(:postgresql)).to eq( + Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::ExecQueryAdapterPatch + ) + end + end + it 'returns Instrumentation::ExecuteAndClearAdapterPatch' do expect(described_class.prepended_class_name(:postgresql)).to eq( Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::ExecuteAndClearAdapterPatch From a2e8ab487dd6a2d77df436be53b8d5726f32ef53 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Wed, 27 Nov 2024 13:48:46 +0100 Subject: [PATCH 6/9] Rename .detect_sqli to .detect_sql_injection --- .../appsec/contrib/active_record/instrumentation.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/datadog/appsec/contrib/active_record/instrumentation.rb b/lib/datadog/appsec/contrib/active_record/instrumentation.rb index e8103410e35..85ca0fc595d 100644 --- a/lib/datadog/appsec/contrib/active_record/instrumentation.rb +++ b/lib/datadog/appsec/contrib/active_record/instrumentation.rb @@ -8,7 +8,7 @@ module ActiveRecord module Instrumentation module_function - def detect_sqli(sql, adapter_name) + def detect_sql_injection(sql, adapter_name) scope = AppSec.active_scope return unless scope @@ -37,7 +37,7 @@ def detect_sqli(sql, adapter_name) # patch for all adapters in ActiveRecord >= 7.1 module InternalExecQueryAdapterPatch def internal_exec_query(sql, *args, **rest) - Instrumentation.detect_sqli(sql, adapter_name) + Instrumentation.detect_sql_injection(sql, adapter_name) super end @@ -46,7 +46,7 @@ def internal_exec_query(sql, *args, **rest) # patch for postgres adapter in ActiveRecord < 7.1 module ExecuteAndClearAdapterPatch def execute_and_clear(sql, *args, **rest) - Instrumentation.detect_sqli(sql, adapter_name) + Instrumentation.detect_sql_injection(sql, adapter_name) super end @@ -56,7 +56,7 @@ def execute_and_clear(sql, *args, **rest) # this patch is also used when using JDBC adapter module ExecQueryAdapterPatch def exec_query(sql, *args, **rest) - Instrumentation.detect_sqli(sql, adapter_name) + Instrumentation.detect_sql_injection(sql, adapter_name) super end From 776d405fba6b9afe4f53eb792595f9a8887485f7 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Wed, 27 Nov 2024 16:27:40 +0100 Subject: [PATCH 7/9] Add specs for AppSec ActiveRecord instrumentation --- .../appsec/contrib/active_record/multi_adapter_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb b/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb index 7ce35364975..4080ec0e426 100644 --- a/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb +++ b/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb @@ -69,6 +69,16 @@ active_record_scope.to_a end + + it 'adds an event to processor context if waf status is :match' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).and_return(double(status: :match, actions: {})) + ) + + expect(Datadog::AppSec.active_scope.processor_context.events).to receive(:<<).and_call_original + + active_record_scope.to_a + end end context 'mysql2 adapter' do From ee4c3a204f084f5733950a6699e33c823ffa0fc0 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Fri, 29 Nov 2024 15:23:13 +0100 Subject: [PATCH 8/9] Simplify AppSec ActiveRecord patcher and split specs by adapter --- .../contrib/active_record/instrumentation.rb | 7 +- .../contrib/active_record/integration.rb | 2 +- .../appsec/contrib/active_record/patcher.rb | 32 ++-- .../contrib/active_record/instrumentation.rbs | 2 +- .../appsec/contrib/active_record/patcher.rbs | 2 - .../active_record/multi_adapter_spec.rb | 171 ------------------ .../active_record/mysql2_adapter_spec.rb | 106 +++++++++++ .../contrib/active_record/patcher_spec.rb | 70 ------- .../active_record/postgresql_adapter_spec.rb | 113 ++++++++++++ .../active_record/sqlite3_adapter_spec.rb | 103 +++++++++++ 10 files changed, 346 insertions(+), 262 deletions(-) delete mode 100644 spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb create mode 100644 spec/datadog/appsec/contrib/active_record/mysql2_adapter_spec.rb delete mode 100644 spec/datadog/appsec/contrib/active_record/patcher_spec.rb create mode 100644 spec/datadog/appsec/contrib/active_record/postgresql_adapter_spec.rb create mode 100644 spec/datadog/appsec/contrib/active_record/sqlite3_adapter_spec.rb diff --git a/lib/datadog/appsec/contrib/active_record/instrumentation.rb b/lib/datadog/appsec/contrib/active_record/instrumentation.rb index 85ca0fc595d..3b226a9aafb 100644 --- a/lib/datadog/appsec/contrib/active_record/instrumentation.rb +++ b/lib/datadog/appsec/contrib/active_record/instrumentation.rb @@ -12,9 +12,14 @@ def detect_sql_injection(sql, adapter_name) scope = AppSec.active_scope return unless scope + # libddwaf expects db system to be lowercase, + # in case of sqlite adapter, libddwaf expects 'sqlite' as db system + db_system = adapter_name.downcase + db_system = 'sqlite' if db_system == 'sqlite3' + ephemeral_data = { 'server.db.statement' => sql, - 'server.db.system' => adapter_name.downcase.gsub(/\d{1}\z/, '') + 'server.db.system' => db_system } waf_timeout = Datadog.configuration.appsec.waf_timeout diff --git a/lib/datadog/appsec/contrib/active_record/integration.rb b/lib/datadog/appsec/contrib/active_record/integration.rb index 022067d4e19..00002f491d8 100644 --- a/lib/datadog/appsec/contrib/active_record/integration.rb +++ b/lib/datadog/appsec/contrib/active_record/integration.rb @@ -7,7 +7,7 @@ module Datadog module AppSec module Contrib module ActiveRecord - # Description of ActiveRecord integration + # This class provides helper methods that are used when patching ActiveRecord class Integration include Datadog::AppSec::Contrib::Integration diff --git a/lib/datadog/appsec/contrib/active_record/patcher.rb b/lib/datadog/appsec/contrib/active_record/patcher.rb index b6e65b97d51..dd0c6c220ae 100644 --- a/lib/datadog/appsec/contrib/active_record/patcher.rb +++ b/lib/datadog/appsec/contrib/active_record/patcher.rb @@ -23,27 +23,27 @@ def target_version def patch ActiveSupport.on_load :active_record do - if defined? ::ActiveRecord::ConnectionAdapters::SQLite3Adapter - ::ActiveRecord::ConnectionAdapters::SQLite3Adapter.prepend(Patcher.prepended_class_name(:sqlite3)) + instrumentation_module = if ::ActiveRecord.gem_version >= Gem::Version.new('7.1') + Instrumentation::InternalExecQueryAdapterPatch + else + Instrumentation::ExecQueryAdapterPatch + end + + if defined?(::ActiveRecord::ConnectionAdapters::SQLite3Adapter) + ::ActiveRecord::ConnectionAdapters::SQLite3Adapter.prepend(instrumentation_module) end - if defined? ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter - ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(Patcher.prepended_class_name(:postgresql)) + if defined?(::ActiveRecord::ConnectionAdapters::Mysql2Adapter) + ::ActiveRecord::ConnectionAdapters::Mysql2Adapter.prepend(instrumentation_module) end - if defined? ::ActiveRecord::ConnectionAdapters::Mysql2Adapter - ::ActiveRecord::ConnectionAdapters::Mysql2Adapter.prepend(Patcher.prepended_class_name(:mysql2)) - end - end - end + if defined?(::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter) + unless defined?(::ActiveRecord::ConnectionAdapters::JdbcAdapter) + instrumentation_module = Instrumentation::ExecuteAndClearAdapterPatch + end - def prepended_class_name(adapter_name) - if ::ActiveRecord.gem_version >= Gem::Version.new('7.1') - Instrumentation::InternalExecQueryAdapterPatch - elsif adapter_name == :postgresql && !defined?(::ActiveRecord::ConnectionAdapters::JdbcAdapter) - Instrumentation::ExecuteAndClearAdapterPatch - else - Instrumentation::ExecQueryAdapterPatch + ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(instrumentation_module) + end end end end diff --git a/sig/datadog/appsec/contrib/active_record/instrumentation.rbs b/sig/datadog/appsec/contrib/active_record/instrumentation.rbs index b81b4138642..daea0a9f684 100644 --- a/sig/datadog/appsec/contrib/active_record/instrumentation.rbs +++ b/sig/datadog/appsec/contrib/active_record/instrumentation.rbs @@ -3,7 +3,7 @@ module Datadog module Contrib module ActiveRecord module Instrumentation - def self?.detect_sqli: (String sql, String adapter_name) -> void + def self?.detect_sql_injection: (String sql, String adapter_name) -> void module InternalExecQueryAdapterPatch def internal_exec_query: (String sql, *untyped args, **untyped rest) -> untyped diff --git a/sig/datadog/appsec/contrib/active_record/patcher.rbs b/sig/datadog/appsec/contrib/active_record/patcher.rbs index 609b75bf6fb..2850b974c00 100644 --- a/sig/datadog/appsec/contrib/active_record/patcher.rbs +++ b/sig/datadog/appsec/contrib/active_record/patcher.rbs @@ -10,8 +10,6 @@ module Datadog def self?.target_version: () -> Gem::Version? def self?.patch: () -> void - - def self?.prepended_class_name: (Symbol adapter_name) -> class end end end diff --git a/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb b/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb deleted file mode 100644 index 4080ec0e426..00000000000 --- a/spec/datadog/appsec/contrib/active_record/multi_adapter_spec.rb +++ /dev/null @@ -1,171 +0,0 @@ -require 'datadog/appsec/spec_helper' -require 'active_record' - -require 'spec/datadog/tracing/contrib/rails/support/deprecation' - -if PlatformHelpers.jruby? - require 'activerecord-jdbc-adapter' -else - require 'mysql2' - require 'sqlite3' - require 'pg' -end - -RSpec.describe 'AppSec ActiveRecord integration' do - let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } - let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) } - let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset, telemetry: telemetry) } - let(:context) { processor.new_context } - - let(:span) { Datadog::Tracing::SpanOperation.new('root') } - let(:trace) { Datadog::Tracing::TraceOperation.new } - - let!(:user_class) do - stub_const('User', Class.new(ActiveRecord::Base)).tap do |klass| - klass.establish_connection(db_config) - - klass.connection.create_table 'users', force: :cascade do |t| - t.string :name, null: false - t.string :email, null: false - t.timestamps - end - - # prevent internal sql requests from showing up - klass.count - klass.first - end - end - - before do - Datadog.configure do |c| - c.appsec.enabled = true - c.appsec.instrument :active_record - end - - Datadog::AppSec::Scope.activate_scope(trace, span, processor) - - raise_on_rails_deprecation! - end - - after do - Datadog.configuration.reset! - - Datadog::AppSec::Scope.deactivate_scope - processor.finalize - end - - shared_examples 'calls_waf_with_correct_arguments' do - it 'calls waf with correct arguments' do - expect(Datadog::AppSec.active_scope.processor_context).to( - receive(:run).with( - {}, - { - 'server.db.statement' => expected_db_statement, - 'server.db.system' => expected_db_system - }, - Datadog.configuration.appsec.waf_timeout - ).and_call_original - ) - - active_record_scope.to_a - end - - it 'adds an event to processor context if waf status is :match' do - expect(Datadog::AppSec.active_scope.processor_context).to( - receive(:run).and_return(double(status: :match, actions: {})) - ) - - expect(Datadog::AppSec.active_scope.processor_context.events).to receive(:<<).and_call_original - - active_record_scope.to_a - end - end - - context 'mysql2 adapter' do - let(:db_config) do - { - adapter: 'mysql2', - database: ENV.fetch('TEST_MYSQL_DB', 'mysql'), - host: ENV.fetch('TEST_MYSQL_HOST', '127.0.0.1'), - password: ENV.fetch('TEST_MYSQL_ROOT_PASSWORD', 'root'), - port: ENV.fetch('TEST_MYSQL_PORT', '3306') - } - end - - let(:expected_db_system) { 'mysql' } - - context 'when using .where' do - let(:active_record_scope) { User.where(name: 'Bob') } - let(:expected_db_statement) { "SELECT `users`.* FROM `users` WHERE `users`.`name` = 'Bob'" } - - include_examples 'calls_waf_with_correct_arguments' - end - - context 'when using .find_by_sql' do - let(:active_record_scope) { User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'") } - let(:expected_db_statement) { "SELECT * FROM users WHERE name = 'Bob'" } - - include_examples 'calls_waf_with_correct_arguments' - end - end - - context 'postgres adapter' do - let(:db_config) do - { - adapter: 'postgresql', - database: ENV.fetch('TEST_POSTGRES_DB', 'postgres'), - host: ENV.fetch('TEST_POSTGRES_HOST', '127.0.0.1'), - port: ENV.fetch('TEST_POSTGRES_PORT', 5432), - username: ENV.fetch('TEST_POSTGRES_USER', 'postgres'), - password: ENV.fetch('TEST_POSTGRES_PASSWORD', 'postgres') - } - end - - let(:expected_db_system) { 'postgresql' } - - context 'when using .where' do - let(:active_record_scope) { User.where(name: 'Bob') } - let(:expected_db_statement) do - if PlatformHelpers.jruby? - 'SELECT "users".* FROM "users" WHERE "users"."name" = ?' - else - 'SELECT "users".* FROM "users" WHERE "users"."name" = $1' - end - end - - include_examples 'calls_waf_with_correct_arguments' - end - - context 'when using .find_by_sql' do - let(:active_record_scope) { User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'") } - let(:expected_db_statement) { "SELECT * FROM users WHERE name = 'Bob'" } - - include_examples 'calls_waf_with_correct_arguments' - end - end - - context 'sqlite3 adapter' do - let(:db_config) do - { - adapter: 'sqlite3', - database: ':memory:' - } - end - - let(:expected_db_system) { 'sqlite' } - - context 'when using .where' do - let(:active_record_scope) { User.where(name: 'Bob') } - let(:expected_db_statement) { 'SELECT "users".* FROM "users" WHERE "users"."name" = ?' } - - include_examples 'calls_waf_with_correct_arguments' - end - - context 'when using .find_by_sql' do - let(:active_record_scope) { User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'") } - let(:expected_db_statement) { "SELECT * FROM users WHERE name = 'Bob'" } - - include_examples 'calls_waf_with_correct_arguments' - end - end -end diff --git a/spec/datadog/appsec/contrib/active_record/mysql2_adapter_spec.rb b/spec/datadog/appsec/contrib/active_record/mysql2_adapter_spec.rb new file mode 100644 index 00000000000..6c5777adc1f --- /dev/null +++ b/spec/datadog/appsec/contrib/active_record/mysql2_adapter_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'datadog/appsec/spec_helper' +require 'active_record' + +require 'spec/datadog/tracing/contrib/rails/support/deprecation' + +if PlatformHelpers.jruby? + require 'activerecord-jdbc-adapter' +else + require 'mysql2' +end + +RSpec.describe 'AppSec ActiveRecord integration for Mysql2 adapter' do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) } + let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset, telemetry: telemetry) } + let(:context) { processor.new_context } + + let(:span) { Datadog::Tracing::SpanOperation.new('root') } + let(:trace) { Datadog::Tracing::TraceOperation.new } + + let!(:user_class) do + stub_const('User', Class.new(ActiveRecord::Base)).tap do |klass| + klass.establish_connection(db_config) + + klass.connection.create_table 'users', force: :cascade do |t| + t.string :name, null: false + t.string :email, null: false + t.timestamps + end + + # prevent internal sql requests from showing up + klass.count + klass.first + end + end + + let(:db_config) do + { + adapter: 'mysql2', + database: ENV.fetch('TEST_MYSQL_DB', 'mysql'), + host: ENV.fetch('TEST_MYSQL_HOST', '127.0.0.1'), + password: ENV.fetch('TEST_MYSQL_ROOT_PASSWORD', 'root'), + port: ENV.fetch('TEST_MYSQL_PORT', '3306') + } + end + + before do + Datadog.configure do |c| + c.appsec.enabled = true + c.appsec.instrument :active_record + end + + Datadog::AppSec::Scope.activate_scope(trace, span, processor) + + raise_on_rails_deprecation! + end + + after do + Datadog.configuration.reset! + + Datadog::AppSec::Scope.deactivate_scope + processor.finalize + end + + it 'calls waf with correct arguments when querying using .where' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).with( + {}, + { + 'server.db.statement' => "SELECT `users`.* FROM `users` WHERE `users`.`name` = 'Bob'", + 'server.db.system' => 'mysql2' + }, + Datadog.configuration.appsec.waf_timeout + ).and_call_original + ) + + User.where(name: 'Bob').to_a + end + + it 'calls waf with correct arguments when querying using .find_by_sql' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).with( + {}, + { + 'server.db.statement' => "SELECT * FROM users WHERE name = 'Bob'", + 'server.db.system' => 'mysql2' + }, + Datadog.configuration.appsec.waf_timeout + ).and_call_original + ) + + User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'").to_a + end + + it 'adds an event to processor context if waf status is :match' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).and_return(instance_double(Datadog::AppSec::WAF::Result, status: :match, actions: {})) + ) + + expect(Datadog::AppSec.active_scope.processor_context.events).to receive(:<<).and_call_original + + User.where(name: 'Bob').to_a + end +end diff --git a/spec/datadog/appsec/contrib/active_record/patcher_spec.rb b/spec/datadog/appsec/contrib/active_record/patcher_spec.rb deleted file mode 100644 index 468b9631a15..00000000000 --- a/spec/datadog/appsec/contrib/active_record/patcher_spec.rb +++ /dev/null @@ -1,70 +0,0 @@ -# frozen_string_literal: true - -require 'datadog/appsec/spec_helper' -require 'datadog/appsec/contrib/active_record/patcher' - -RSpec.describe Datadog::AppSec::Contrib::ActiveRecord::Patcher do - describe '#prepended_class_name' do - context 'when ActiveRecord version is 7.1 or higher' do - before do - stub_const( - '::ActiveRecord', - Module.new do - module_function def gem_version - Gem::Version.new('7.1') - end - end - ) - end - - it 'returns Instrumentation::InternalExecQueryAdapterPatch' do - expect(described_class.prepended_class_name(:postgresql)).to eq( - Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::InternalExecQueryAdapterPatch - ) - end - end - - context 'when ActiveRecord version is lower than 7.1' do - before do - stub_const( - '::ActiveRecord', - Module.new do - module_function def gem_version - Gem::Version.new('7.0') - end - end - ) - end - - context 'for postgresql adapter' do - context 'when ActiveRecord::ConnectionAdapters::JdbcAdapter is defined' do - before do - stub_const('::ActiveRecord::ConnectionAdapters::JdbcAdapter', Class.new) - end - - it 'returns Instrumentation::ExecQueryAdapterPatch' do - expect(described_class.prepended_class_name(:postgresql)).to eq( - Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::ExecQueryAdapterPatch - ) - end - end - - it 'returns Instrumentation::ExecuteAndClearAdapterPatch' do - expect(described_class.prepended_class_name(:postgresql)).to eq( - Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::ExecuteAndClearAdapterPatch - ) - end - end - - %i[mysql2 sqlite3].each do |adapter_name| - context "for #{adapter_name} adapter" do - it 'returns Instrumentation::ExecQueryAdapterPatch' do - expect(described_class.prepended_class_name(adapter_name)).to eq( - Datadog::AppSec::Contrib::ActiveRecord::Instrumentation::ExecQueryAdapterPatch - ) - end - end - end - end - end -end diff --git a/spec/datadog/appsec/contrib/active_record/postgresql_adapter_spec.rb b/spec/datadog/appsec/contrib/active_record/postgresql_adapter_spec.rb new file mode 100644 index 00000000000..5ebd8e0cacb --- /dev/null +++ b/spec/datadog/appsec/contrib/active_record/postgresql_adapter_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'datadog/appsec/spec_helper' +require 'active_record' + +require 'spec/datadog/tracing/contrib/rails/support/deprecation' + +if PlatformHelpers.jruby? + require 'activerecord-jdbc-adapter' +else + require 'pg' +end + +RSpec.describe 'AppSec ActiveRecord integration for Postgresql adapter' do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) } + let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset, telemetry: telemetry) } + let(:context) { processor.new_context } + + let(:span) { Datadog::Tracing::SpanOperation.new('root') } + let(:trace) { Datadog::Tracing::TraceOperation.new } + + let!(:user_class) do + stub_const('User', Class.new(ActiveRecord::Base)).tap do |klass| + klass.establish_connection(db_config) + + klass.connection.create_table 'users', force: :cascade do |t| + t.string :name, null: false + t.string :email, null: false + t.timestamps + end + + # prevent internal sql requests from showing up + klass.count + klass.first + end + end + + let(:db_config) do + { + adapter: 'postgresql', + database: ENV.fetch('TEST_POSTGRES_DB', 'postgres'), + host: ENV.fetch('TEST_POSTGRES_HOST', '127.0.0.1'), + port: ENV.fetch('TEST_POSTGRES_PORT', 5432), + username: ENV.fetch('TEST_POSTGRES_USER', 'postgres'), + password: ENV.fetch('TEST_POSTGRES_PASSWORD', 'postgres') + } + end + + before do + Datadog.configure do |c| + c.appsec.enabled = true + c.appsec.instrument :active_record + end + + Datadog::AppSec::Scope.activate_scope(trace, span, processor) + + raise_on_rails_deprecation! + end + + after do + Datadog.configuration.reset! + + Datadog::AppSec::Scope.deactivate_scope + processor.finalize + end + + it 'calls waf with correct arguments when querying using .where' do + expected_db_statement = if PlatformHelpers.jruby? + 'SELECT "users".* FROM "users" WHERE "users"."name" = ?' + else + 'SELECT "users".* FROM "users" WHERE "users"."name" = $1' + end + + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).with( + {}, + { + 'server.db.statement' => expected_db_statement, + 'server.db.system' => 'postgresql' + }, + Datadog.configuration.appsec.waf_timeout + ).and_call_original + ) + + User.where(name: 'Bob').to_a + end + + it 'calls waf with correct arguments when querying using .find_by_sql' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).with( + {}, + { + 'server.db.statement' => "SELECT * FROM users WHERE name = 'Bob'", + 'server.db.system' => 'postgresql' + }, + Datadog.configuration.appsec.waf_timeout + ).and_call_original + ) + + User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'").to_a + end + + it 'adds an event to processor context if waf status is :match' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).and_return(instance_double(Datadog::AppSec::WAF::Result, status: :match, actions: {})) + ) + + expect(Datadog::AppSec.active_scope.processor_context.events).to receive(:<<).and_call_original + + User.where(name: 'Bob').to_a + end +end diff --git a/spec/datadog/appsec/contrib/active_record/sqlite3_adapter_spec.rb b/spec/datadog/appsec/contrib/active_record/sqlite3_adapter_spec.rb new file mode 100644 index 00000000000..778fe952a30 --- /dev/null +++ b/spec/datadog/appsec/contrib/active_record/sqlite3_adapter_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'datadog/appsec/spec_helper' +require 'active_record' + +require 'spec/datadog/tracing/contrib/rails/support/deprecation' + +if PlatformHelpers.jruby? + require 'activerecord-jdbc-adapter' +else + require 'sqlite3' +end + +RSpec.describe 'AppSec ActiveRecord integration for SQLite3 adapter' do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) } + let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset, telemetry: telemetry) } + let(:context) { processor.new_context } + + let(:span) { Datadog::Tracing::SpanOperation.new('root') } + let(:trace) { Datadog::Tracing::TraceOperation.new } + + let!(:user_class) do + stub_const('User', Class.new(ActiveRecord::Base)).tap do |klass| + klass.establish_connection(db_config) + + klass.connection.create_table 'users', force: :cascade do |t| + t.string :name, null: false + t.string :email, null: false + t.timestamps + end + + # prevent internal sql requests from showing up + klass.count + klass.first + end + end + + let(:db_config) do + { + adapter: 'sqlite3', + database: ':memory:' + } + end + + before do + Datadog.configure do |c| + c.appsec.enabled = true + c.appsec.instrument :active_record + end + + Datadog::AppSec::Scope.activate_scope(trace, span, processor) + + raise_on_rails_deprecation! + end + + after do + Datadog.configuration.reset! + + Datadog::AppSec::Scope.deactivate_scope + processor.finalize + end + + it 'calls waf with correct arguments when querying using .where' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).with( + {}, + { + 'server.db.statement' => 'SELECT "users".* FROM "users" WHERE "users"."name" = ?', + 'server.db.system' => 'sqlite' + }, + Datadog.configuration.appsec.waf_timeout + ).and_call_original + ) + + User.where(name: 'Bob').to_a + end + + it 'calls waf with correct arguments when querying using .find_by_sql' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).with( + {}, + { + 'server.db.statement' => "SELECT * FROM users WHERE name = 'Bob'", + 'server.db.system' => 'sqlite' + }, + Datadog.configuration.appsec.waf_timeout + ).and_call_original + ) + + User.find_by_sql("SELECT * FROM users WHERE name = 'Bob'").to_a + end + + it 'adds an event to processor context if waf status is :match' do + expect(Datadog::AppSec.active_scope.processor_context).to( + receive(:run).and_return(instance_double(Datadog::AppSec::WAF::Result, status: :match, actions: {})) + ) + + expect(Datadog::AppSec.active_scope.processor_context.events).to receive(:<<).and_call_original + + User.where(name: 'Bob').to_a + end +end From 971f329405bcb72670e77b6121933e22c9d6a651 Mon Sep 17 00:00:00 2001 From: Yury Lebedev Date: Mon, 2 Dec 2024 11:31:55 +0100 Subject: [PATCH 9/9] Move require of AppSec active_record instrumentation --- lib/datadog/appsec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index 7341af4c790..5f96dbdd5fa 100644 --- a/lib/datadog/appsec.rb +++ b/lib/datadog/appsec.rb @@ -53,10 +53,10 @@ def components end # Integrations -require_relative 'appsec/contrib/active_record/integration' require_relative 'appsec/contrib/rack/integration' require_relative 'appsec/contrib/sinatra/integration' require_relative 'appsec/contrib/rails/integration' +require_relative 'appsec/contrib/active_record/integration' require_relative 'appsec/contrib/devise/integration' require_relative 'appsec/contrib/graphql/integration'