Request session id unavailable when streaming HTML

joeldrapper commented 5 months ago

The default Content Security Policy nonce generator returns the request.session.id, but this isn’t available when streaming an HTML response.

Steps to reproduce

Enable the default Content Security Policy by uncommenting the code generated in config/initializers/content_security_policy.rb
Stream an HTML response form a controller

Content Security Policy Config:

Rails.application.configure do
  config.content_security_policy do |policy|
    policy.default_src :self, :https
    policy.font_src    :self, :https, :data
    policy.img_src     :self, :https, :data
    policy.object_src  :none
    policy.script_src  :self, :https
    policy.style_src   :self, :https
  end

  config.content_security_policy_nonce_generator = ->(request) { request.session.id }
  config.content_security_policy_nonce_directives = %w(script-src style-src)
end

Example controller:

class DashboardController < ApplicationController
  def index
    headers["X-Accel-Buffering"] = "no"
    headers["Cache-Control"] = "no-transform"
    headers["Last-Modified"] = Time.now.httpdate
    headers["Content-Type"] = "text/html; charset=utf-8"

    self.response_body = Enumerator.new do |buffer|
      buffer << "<html><head><title>Dashboard</title></head><body>"
      buffer << "<h1>Dashboard</h1>"
      buffer << "<p>Welcome to the dashboard</p>"
      buffer << "</body></html>"
    end
  end
end

Expected behaviour

The Content Security Policy headers should include a nonce based on the session id.

Content-Security-Policy: default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src 'self' https: 'nonce-2c3f06bf498ed7295cc4612bdc7ad90e'; style-src 'self' https: 'nonce-2c3f06bf498ed7295cc4612bdc7ad90e'

Actual behaviour

The Content Security Policy headers do not include a nonce.

Content-Security-Policy: default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src 'self' https:; style-src 'self' https:

Workaround

As a workaround, you can set a session attribute and remove it before setting the response_body.

session[:foo] = 1
session.delete(:foo)

This seems to get the session into a state where it has an id.

System configuration

Rails version: 7.1.3.2

Ruby version: 3.3.0

justinko commented 3 months ago

Can reproduce with 7.1.3.3 but not main.

zzak commented 3 months ago

@justinko Does it happen on 7-1-stable branch? Would you be able to create a repro script and/or bisect it?

justinko commented 3 months ago

Repro script ... and it fails with main:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails", path: ".."
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"
end

require "action_controller/railtie"

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts << "example.org"
  config.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  routes.draw do
    get "/" => "test#index"
  end

  config.content_security_policy do |policy|
    policy.default_src :self, :https
    policy.font_src    :self, :https, :data
    policy.img_src     :self, :https, :data
    policy.object_src  :none
    policy.script_src  :self, :https
    policy.style_src   :self, :https
  end

  config.content_security_policy_nonce_generator = ->(request) { request.session.id }
  config.content_security_policy_nonce_directives = %w(script-src style-src)
  config.session_store :cookie_store
end

class TestController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    headers["X-Accel-Buffering"] = "no"
    headers["Cache-Control"] = "no-transform"
    headers["Last-Modified"] = Time.now.httpdate
    headers["Content-Type"] = "text/html; charset=utf-8"

    # session[:foo] = 1
    # session.delete(:foo)

    self.response_body = Enumerator.new do |buffer|
      buffer << "<html><head><title>Dashboard</title></head><body>"
      buffer << "<h1>Dashboard</h1>"
      buffer << "<p>Welcome to the dashboard</p>"
      buffer << "</body></html>"
    end
  end
end

require "minitest/autorun"
require "rack/test"

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_returns_success
    get "/"
    assert last_response.ok?
    refute last_response.headers["content-security-policy"].end_with?("https:")
  end

  private
    def app
      Rails.application
    end
end

Will try and narrow down a commit now.

justinko commented 3 months ago

Everything is working as intended -- no regression (from what I can see).

The reason why session.id is nil is because the session hasn't been loaded yet, which only occurs on some of its methods.

I'm guessing the error doesn't occur "in the wild" because if you're accessing a session, then most likely you're also setting values on it or interacting with it some way other than simply calling id.

With your example, you can call request.session.send(:load!) before calling id to resolve the issue.

I tried loading the session when you call id but there's a circular dependency with rack-session that doesn't make it worth it.

joeldrapper commented 3 months ago

It doesn’t seem right to have to call a private method on the request session in order to use the default content security configuration.

justinko commented 3 months ago

I'll take a stab at an implementation.

rails-bot[bot] commented 1 week ago

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-2-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

rails / rails