Merge pull request #5042 from DataDog/fix-api-security-route-extractor-for-rails-8-1-1

y9v · web-flow · commit e4ed4400eb7d · 2025-11-13T10:25:57.000+01:00
Fix APISecurity::RouteExtractor for Rails 8.1.1+
diff --git a/lib/datadog/appsec/api_security/route_extractor.rb b/lib/datadog/appsec/api_security/route_extractor.rb
@@ -10,7 +10,8 @@ module RouteExtractor
         SINATRA_ROUTE_KEY = 'sinatra.route'
         SINATRA_ROUTE_SEPARATOR = ' '
         GRAPE_ROUTE_KEY = 'grape.routing_args'
-        RAILS_ROUTE_KEY = 'action_dispatch.route_uri_pattern'
+        RAILS_ROUTE_URI_PATTERN_KEY = 'action_dispatch.route_uri_pattern'
+        RAILS_ROUTE_KEY = 'action_dispatch.route' # Rails 8.1.1+
         RAILS_ROUTES_KEY = 'action_dispatch.routes'
         RAILS_PATH_PARAMS_KEY = 'action_dispatch.request.path_parameters'
         RAILS_FORMAT_SUFFIX = '(.:format)'
@@ -37,6 +38,9 @@ module RouteExtractor
         #       Rails > 7.1 (fast path)
         #         uses `action_dispatch.route_uri_pattern` with a string like
         #         "/users/:id(.:format)"
+        #       Rails > 8.1.1 (fast path)
+        #         uses `action_dispatch.route` to store the ActionDispatch::Journey::Route
+        #         that matched when the request was routed
         #
         # WARNING: This method works only *after* the request has been routed.
         #
@@ -52,11 +56,18 @@ def self.route_pattern(request)
             pattern = request.env[SINATRA_ROUTE_KEY].split(SINATRA_ROUTE_SEPARATOR, 2)[1]
             "#{request.script_name}#{pattern}"
           elsif request.env.key?(RAILS_ROUTE_KEY)
-            request.env[RAILS_ROUTE_KEY].delete_suffix(RAILS_FORMAT_SUFFIX)
+            request.env[RAILS_ROUTE_KEY].path.spec.to_s.delete_suffix(RAILS_FORMAT_SUFFIX)
+          elsif request.env.key?(RAILS_ROUTE_URI_PATTERN_KEY)
+            request.env[RAILS_ROUTE_URI_PATTERN_KEY].delete_suffix(RAILS_FORMAT_SUFFIX)
           elsif request.env.key?(RAILS_ROUTES_KEY) && !request.env.fetch(RAILS_PATH_PARAMS_KEY, {}).empty?
-            # NOTE: Rails mutate HEAD request in order to understand that route is supported.
-            #       It will assing GET request method and run the route recognition.
-            request = request.env[RAILS_ROUTES_KEY].request_class.new(request.env) if request.head?
+            # NOTE: In Rails < 7.1 this `request` argument will be a Rack::Request,
+            #       it does not have all the methods that ActionDispatch::Request has.
+            #       Before trying to use the router to recognize the route, we need to
+            #       create a new ActionDispatch::Request from the request env
+            #
+            # NOTE: Rails mutates HEAD request by changing the method to GET
+            #       and uses it for route recognition to check if the route is defined
+            request = request.env[RAILS_ROUTES_KEY].request_class.new(request.env)
 
             pattern = request.env[RAILS_ROUTES_KEY].router
               .recognize(request) { |route, _| break route.path.spec.to_s }
@@ -70,6 +81,10 @@ def self.route_pattern(request)
           else
             Tracing::Contrib::Rack::RouteInference.read_or_infer(request.env)
           end
+        rescue => e
+          AppSec.telemetry&.report(e, description: 'AppSec: Could not extract route pattern')
+
+          nil
         end
       end
     end
diff --git a/lib/datadog/appsec/api_security/sampler.rb b/lib/datadog/appsec/api_security/sampler.rb
@@ -45,7 +45,9 @@ def sample?(request, response)
           return true if @sample_delay_seconds.zero?
           return false if response.status == 404
 
-          key = Zlib.crc32("#{request.request_method}#{RouteExtractor.route_pattern(request)}#{response.status}")
+          route_pattern = RouteExtractor.route_pattern(request).to_s
+
+          key = Zlib.crc32("#{request.request_method}#{route_pattern}#{response.status}")
           current_timestamp = Core::Utils::Time.now.to_i
           cached_timestamp = @cache[key] || 0
 
diff --git a/sig/datadog/appsec/api_security/route_extractor.rbs b/sig/datadog/appsec/api_security/route_extractor.rbs
@@ -10,6 +10,8 @@ module Datadog
 
         RAILS_ROUTE_KEY: String
 
+        RAILS_ROUTE_URI_PATTERN_KEY: String
+
         RAILS_ROUTES_KEY: String
 
         RAILS_PATH_PARAMS_KEY: String
diff --git a/spec/datadog/appsec/api_security/route_extractor_spec.rb b/spec/datadog/appsec/api_security/route_extractor_spec.rb
@@ -68,31 +68,61 @@
     end
 
     context 'when Rails routing is present' do
-      context 'when route has format suffix' do
-        before do
-          allow(request).to receive(:env).and_return({'action_dispatch.route_uri_pattern' => '/users/:id(.:format)'})
+      context 'when action_dispatch.route_uri_pattern is set' do
+        context 'when route has format suffix' do
+          before do
+            allow(request).to receive(:env).and_return({'action_dispatch.route_uri_pattern' => '/users/:id(.:format)'})
+          end
+
+          it { expect(described_class.route_pattern(request)).to eq('/users/:id') }
         end
 
-        it { expect(described_class.route_pattern(request)).to eq('/users/:id') }
-      end
+        context 'when route has no format suffix' do
+          before { allow(request).to receive(:env).and_return({'action_dispatch.route_uri_pattern' => '/users/:id'}) }
 
-      context 'when route has no format suffix' do
-        before { allow(request).to receive(:env).and_return({'action_dispatch.route_uri_pattern' => '/users/:id'}) }
+          it { expect(described_class.route_pattern(request)).to eq('/users/:id') }
+        end
 
-        it { expect(described_class.route_pattern(request)).to eq('/users/:id') }
+        context 'when route has nested path' do
+          before do
+            allow(request).to receive(:env).and_return(
+              {'action_dispatch.route_uri_pattern' => '/api/v1/users/:id/posts/:post_id(.:format)'}
+            )
+          end
+
+          it { expect(described_class.route_pattern(request)).to eq('/api/v1/users/:id/posts/:post_id') }
+        end
       end
 
-      context 'when route has nested path' do
-        before do
-          allow(request).to receive(:env).and_return(
-            {'action_dispatch.route_uri_pattern' => '/api/v1/users/:id/posts/:post_id(.:format)'}
-          )
+      context 'when action_dispatch.route is set (Rails 8.1.1 and up)' do
+        context 'when route has format suffix' do
+          let(:route_double) do
+            spec_double = instance_double('ActionDispatch::Journey::Nodes::Cat', to_s: '/users/:id(.:format)')
+            path_double = instance_double('ActionDispatch::Journey::Path::Pattern', spec: spec_double)
+            instance_double('ActionDispatch::Journey::Route', path: path_double)
+          end
+
+          before do
+            allow(request).to receive(:env).and_return({'action_dispatch.route' => route_double})
+          end
+
+          it { expect(described_class.route_pattern(request)).to eq('/users/:id') }
         end
 
-        it { expect(described_class.route_pattern(request)).to eq('/api/v1/users/:id/posts/:post_id') }
+        context 'when route has no format suffix' do
+          let(:route_double) do
+            spec_double = instance_double('ActionDispatch::Journey::Nodes::Cat', to_s: '/users/:id')
+            path_double = instance_double('ActionDispatch::Journey::Path::Pattern', spec: spec_double)
+            instance_double('ActionDispatch::Journey::Route', path: path_double)
+          end
+
+          before { allow(request).to receive(:env).and_return({'action_dispatch.route' => route_double}) }
+
+          it { expect(described_class.route_pattern(request)).to eq('/users/:id') }
+        end
       end
 
-      context 'when route_uri_pattern is not set and request path_parameters is empty' do
+      context 'when neither route or route_uri_pattern is set and request path_parameters are empty' do
         before do
           allow(request).to receive(:env).and_return({
             'action_dispatch.routes' => route_set,
@@ -114,7 +144,7 @@
         end
       end
 
-      context 'when route_uri_pattern is not set and request path_parameters is present' do
+      context 'when neither route route_uri_pattern is set and request path_parameters are present' do
         let(:env) do
           {
             'action_dispatch.routes' => route_set,
@@ -123,18 +153,18 @@
             }
           }
         end
+        let(:router) { double('ActionDispatch::Routing::RouteSet::Router') }
+        let(:route_set) { double('ActionDispatch::Routing::RouteSet', router: router, request_class: action_dispatch_request_class) }
+        let(:action_dispatch_request_class) { double('class ActionDispatch::Request', new: action_dispatch_request) }
+        let(:action_dispatch_request) { double('ActionDispatch::Request', env: {}, script_name: '', path: '/users/1') }
 
-        context 'when request is HEAD' do
-          before do
-            allow(request).to receive(:env).and_return(env)
-            allow(action_dispatch_request).to receive(:env).and_return(env)
-          end
+        before do
+          allow(request).to receive(:env).and_return(env)
+          allow(action_dispatch_request).to receive(:env).and_return(env)
+        end
 
-          let(:router) { double('ActionDispatch::Routing::RouteSet::Router') }
-          let(:route_set) { double('ActionDispatch::Routing::RouteSet', router: router, request_class: action_dispatch_request_class) }
+        context 'when request is HEAD' do
           let(:request) { double('Rack::Request', env: {}, script_name: '', path: '/users/1', head?: true) }
-          let(:action_dispatch_request_class) { double('class ActionDispatch::Request', new: action_dispatch_request) }
-          let(:action_dispatch_request) { double('ActionDispatch::Request', env: {}, script_name: '', path: '/users/1') }
 
           it 'uses action dispatch request for route recognition' do
             expect(router).to receive(:recognize).with(action_dispatch_request).and_return('/users/:id(.:format)')
@@ -143,14 +173,10 @@
         end
 
         context 'when request is not HEAD' do
-          before { allow(request).to receive(:env).and_return(env) }
-
-          let(:router) { double('ActionDispatch::Routing::RouteSet::Router') }
-          let(:route_set) { double('ActionDispatch::Routing::RouteSet', router: router) }
           let(:request) { double('Rack::Request', env: {}, script_name: '', path: '/users/1', head?: false) }
 
           it 'uses action dispatch request for route recognition' do
-            expect(router).to receive(:recognize).with(request).and_return('/users/:id(.:format)')
+            expect(router).to receive(:recognize).with(action_dispatch_request).and_return('/users/:id(.:format)')
             expect(described_class.route_pattern(request)).to eq('/users/:id')
           end
         end
@@ -170,6 +196,35 @@
 
         it { expect(described_class.route_pattern(request)).to eq('/unmatched/route') }
       end
+
+      context 'when an error is raised during route recognition' do
+        before do
+          allow(Datadog::AppSec).to receive(:telemetry).and_return(telemetry)
+
+          allow(request).to receive(:env).and_return({
+            'action_dispatch.routes' => route_set,
+            'action_dispatch.request.path_parameters' => {
+              'controller' => 'users',
+              'action' => 'show',
+              'id' => '1'
+            }
+          })
+
+          expect(route_set).to receive(:request_class).and_raise(StandardError)
+        end
+
+        let(:route_set) { double('ActionDispatch::Routing::RouteSet') }
+        let(:telemetry) { spy(Datadog::Core::Telemetry::Component) }
+
+        it { expect(described_class.route_pattern(request)).to be_nil }
+
+        it 'reports the error via telemetry' do
+          expect(telemetry).to receive(:report)
+            .with(an_instance_of(StandardError), description: 'AppSec: Could not extract route pattern')
+
+          described_class.route_pattern(request)
+        end
+      end
     end
 
     context 'when Rack routing is present' do
