FIX: Improve handling of 'PublicExceptions' when bootstrap_error_pages enabled (#26700)

- Run the CSP-nonce-related middlewares on the generated response

- Fix the readonly mode checking to avoid empty strings being passed (the `check_readonly_mode` before_action will not execute in the case of these re-dispatched exceptions)

- Move the BlockRequestsMiddleware cookie-setting to the middleware, so that it is included even for unusual HTML responses like these exceptions

app/controllers/application_controller.rb

@@ -641,8 +641,8 @@ class ApplicationController < ActionController::Base
     store_preloaded("customHTML", custom_html_json)
     store_preloaded("banner", banner_json)
     store_preloaded("customEmoji", custom_emoji)
-    store_preloaded("isReadOnly", @readonly_mode.to_s)
+    store_preloaded("isReadOnly", get_or_check_readonly_mode.to_json)
-    store_preloaded("isStaffWritesOnly", @staff_writes_only_mode.to_s)
+    store_preloaded("isStaffWritesOnly", get_or_check_staff_writes_only_mode.to_json)
     store_preloaded("activatedThemes", activated_themes_json)
   end
 

config/initializers/200-first_middlewares.rb

@@ -62,16 +62,23 @@ if Rails.env.test?
                    request.cookies[RSPEC_CURRENT_EXAMPLE_COOKIE_STRING]
              )
          )
-        [
+        return [
           503,
           { "Content-Type" => "text/plain" },
           [
             "Blocked by BlockRequestsMiddleware for requests initiated by #{request.cookies[RSPEC_CURRENT_EXAMPLE_COOKIE_STRING]} when running #{self.class.current_example_location}",
-          ],
+          ]
         ]
-      else
-        @app.call(env)
       end
+
+      status, headers, body = @app.call(env)
+      if headers["Content-Type"]&.match?(/html/) && BlockRequestsMiddleware.current_example_location
+        headers["Set-Cookie"] = [
+          headers["Set-Cookie"],
+          "#{RSPEC_CURRENT_EXAMPLE_COOKIE_STRING}=#{BlockRequestsMiddleware.current_example_location}; path=/;",
+        ].compact.join("\n")
+      end
+      [status, headers, body]
     end
   end
 

lib/middleware/discourse_public_exceptions.rb

@@ -4,6 +4,12 @@
 # we need to handle certain exceptions here
 module Middleware
   class DiscoursePublicExceptions < ::ActionDispatch::PublicExceptions
+    # These middlewares will be re-run when the exception response is generated
+    EXCEPTION_RESPONSE_MIDDLEWARES = [
+      ContentSecurityPolicy::Middleware,
+      Middleware::CspScriptNonceInjector,
+    ]
+
     INVALID_REQUEST_ERRORS =
       Set.new(
         [
@@ -60,7 +66,10 @@ module Middleware
           if ApplicationController.rescue_with_handler(exception, object: fake_controller)
             body = response.body
             body = [body] if String === body
-            return response.status, response.headers, body
+            rack_response = [response.status, response.headers, body]
+            app = lambda { |env| rack_response }
+            EXCEPTION_RESPONSE_MIDDLEWARES.each { |middleware| app = middleware.new(app) }
+            return app.call(env)
           end
         rescue => e
           return super if INVALID_REQUEST_ERRORS.include?(e.class)

lib/read_only_mixin.rb

@@ -32,6 +32,16 @@ module ReadOnlyMixin
     end
   end
 
+  def get_or_check_readonly_mode
+    check_readonly_mode if @readonly_mode.nil?
+    @readonly_mode
+  end
+
+  def get_or_check_staff_writes_only_mode
+    check_readonly_mode if @readonly_mode.nil?
+    @readonly_mode
+  end
+
   def add_readonly_header
     response.headers["Discourse-Readonly"] = "true" if @readonly_mode
   end

spec/rails_helper.rb

@@ -580,15 +580,6 @@ RSpec.configure do |config|
         ActiveRecord::Base.connection.schema_cache.add(table)
       end
 
-      ApplicationController.before_action(prepend: true) do
-        if BlockRequestsMiddleware.current_example_location && !request.xhr? &&
-             request.format == "html"
-          cookies[
-            BlockRequestsMiddleware::RSPEC_CURRENT_EXAMPLE_COOKIE_STRING
-          ] = BlockRequestsMiddleware.current_example_location
-        end
-      end
-
       system_tests_initialized = true
     end
 

spec/system/bootstrap_error_pages_spec.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+describe "bootstrap_error_pages", type: :system do
+  before { SiteSetting.bootstrap_error_pages = true }
+
+  it "boots ember for non-existent route" do
+    visit "/foobar"
+    expect(page).not_to have_css("body.no-ember")
+    expect(page).to have_css("#site-logo")
+    expect(page).to have_css("div.page-not-found")
+  end
+
+  it "boots ember for non-existent topic" do
+    visit "/t/999999999999"
+    expect(page).not_to have_css("body.no-ember")
+    expect(page).to have_css("#site-logo")
+    expect(page).to have_css("div.page-not-found")
+  end
+end

spec/system/content_security_policy_spec.rb

@@ -9,6 +9,19 @@ describe "Content security policy", type: :system do
     expect(page).to have_css("#site-logo")
   end
 
+  it "works for 'public exceptions' like RoutingError" do
+    expect(SiteSetting.content_security_policy).to eq(true)
+    SiteSetting.content_security_policy_strict_dynamic = true
+    SiteSetting.bootstrap_error_pages = true
+
+    get "/nonexistent"
+    expect(response.headers["Content-Security-Policy"]).to include("'strict-dynamic'")
+
+    visit "/nonexistent"
+    expect(page).not_to have_css("body.no-ember")
+    expect(page).to have_css("#site-logo")
+  end
+
   it "can boot logster in strict_dynamic mode" do
     expect(SiteSetting.content_security_policy).to eq(true)
     sign_in Fabricate(:admin)