From cad2d7211dff02488ec591dcf2161c1039ad5dd1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 16:36:19 -0700 Subject: [PATCH 1/8] Serve uploads that aren't media as downloads Uploaded files come from this origin, so one served with a JavaScript type could run as one of the app's own scripts. Browsers accept sixteen spellings of that type, and Disk URLs sign the type they were minted with, so refusing a list of types at upload can't cover links already out there. Instead, a middleware rewrites the type of anything Active Storage reads out that isn't plainly media, a PDF or plain text to application/octet-stream, whatever the URL says. --- config/initializers/uploaded_script_guard.rb | 31 ++++++++++++++++ test/middleware/uploaded_script_guard_test.rb | 37 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 config/initializers/uploaded_script_guard.rb create mode 100644 test/middleware/uploaded_script_guard_test.rb diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb new file mode 100644 index 0000000..18cf59e --- /dev/null +++ b/config/initializers/uploaded_script_guard.rb @@ -0,0 +1,31 @@ +# Uploaded files are served from this origin, so under script-src 'self' an +# upload served with any JavaScript MIME type would run as one of our scripts. +# Browsers accept sixteen spellings of that type, and Disk URLs sign the type +# they were minted with, so a list of types to refuse can't cover old links. +# Instead, anything Active Storage serves that isn't plainly media goes out as +# a download, whatever its URL says. Only reads are touched: uploading +# (direct upload JSON, Disk PUT) answers in its own types. +# +# Accepted on upgrade: a browser that cached an upload under a script type +# before this guard shipped can still run it from that cache until the entry +# expires, up to an hour after it was fetched (config/initializers/active_storage.rb). +# Revalidation doesn't help, since a 304 carries no content type. +class UploadedScriptGuard + SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain)\z} + + def initialize(app) + @app = app + end + + def call(env) + status, headers, body = @app.call(env) + + if env["REQUEST_METHOD"].in?(%w[ GET HEAD ]) && env["PATH_INFO"].start_with?("#{ActiveStorage.routes_prefix}/") && (type = headers["content-type"]) + headers["content-type"] = ActiveStorage.binary_content_type unless type.split(";").first.strip.downcase.match?(SERVABLE) + end + + [ status, headers, body ] + end +end + +Rails.application.config.middleware.use UploadedScriptGuard diff --git a/test/middleware/uploaded_script_guard_test.rb b/test/middleware/uploaded_script_guard_test.rb new file mode 100644 index 0000000..c627c9f --- /dev/null +++ b/test/middleware/uploaded_script_guard_test.rb @@ -0,0 +1,37 @@ +require "test_helper" + +class UploadedScriptGuardTest < ActiveSupport::TestCase + # Every JavaScript MIME type essence in the MIME Sniffing Standard, plus + # parameters and casing a signed Disk URL might carry. + SCRIPT_TYPES = %w[ + application/ecmascript application/javascript application/x-ecmascript application/x-javascript + text/ecmascript text/javascript text/javascript1.0 text/javascript1.1 text/javascript1.2 text/javascript1.3 + text/javascript1.4 text/javascript1.5 text/jscript text/livescript text/x-ecmascript text/x-javascript + ] + [ "Text/JavaScript; charset=utf-8" ] + + test "uploads never leave with a script type, whatever the URL says" do + SCRIPT_TYPES.each do |type| + assert_equal ActiveStorage.binary_content_type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/probe.js"), type + end + end + + test "media and downloads keep their type" do + %w[ image/png video/mp4 audio/mpeg application/pdf text/plain ].each do |type| + assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/file"), type + end + end + + test "direct upload responses keep their JSON type" do + assert_equal "application/json", served_type("application/json", "#{ActiveStorage.routes_prefix}/direct_uploads", method: "POST") + end + + test "application responses outside uploads are untouched" do + assert_equal "text/javascript", served_type("text/javascript", "/assets/application-abc123.js") + end + + private + def served_type(type, path, method: "GET") + app = ->(env) { [ 200, { "content-type" => type }, [ "" ] ] } + UploadedScriptGuard.new(app).call(Rack::MockRequest.env_for(path, method: method))[1]["content-type"] + end +end From 562888251ed80179cfde9ba97ef35aaa4f8a3749 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 16:36:19 -0700 Subject: [PATCH 2/8] Enforce a script policy that allows the importmap by hash script-src 'self', object-src 'none' and base-uri 'self', enforced. The two inline scripts the importmap helper emits are allowed by their SHA-256, and the origins its URLs point to (a configured asset host, in any form Rails accepts) are allowed by name. There is no per-request nonce, so cached pages stay valid. DISABLE_CONTENT_SECURITY_POLICY=true leaves the policy off, for an install whose proxy or customization injects scripts. --- app/controllers/application_controller.rb | 2 +- .../concerns/inline_script_hashes.rb | 33 ++++++++ .../initializers/content_security_policy.rb | 39 ++++------ test/controllers/cached_responses_test.rb | 12 +++ .../content_security_policy_test.rb | 78 +++++++++++++++++++ 5 files changed, 140 insertions(+), 24 deletions(-) create mode 100644 app/controllers/concerns/inline_script_hashes.rb create mode 100644 test/integration/content_security_policy_test.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9ad6c7c..b73e592 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,4 +1,4 @@ class ApplicationController < ActionController::Base - include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses + include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses, InlineScriptHashes include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName end diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb new file mode 100644 index 0000000..c0fde41 --- /dev/null +++ b/app/controllers/concerns/inline_script_hashes.rb @@ -0,0 +1,33 @@ +# Allows the scripts the importmap helper emits: the inline importmap and its +# entry-point module by content hash, and the origins its URLs point to. The +# hashes change only when the importmap does (a deploy), so cached HTML stays +# valid, unlike a per-request nonce. +module InlineScriptHashes + extend ActiveSupport::Concern + + ENTRY_POINT_SCRIPT = %(import "application") + + included do + content_security_policy do |policy| + if sources = policy.directives["script-src"] + importmap = Rails.application.importmap.to_json(resolver: helpers) + policy.directives["script-src"] = sources | script_origins(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT) + end + end + end + + private + # The importmap is resolved by Rails' asset helpers, so its URLs already + # carry any configured asset host, however it's written: with or without a + # scheme, protocol-relative, a %d set, or a proc. + def script_origins(importmap) + JSON.parse(importmap)["imports"].values.filter_map do |url| + origin = URI.join(request.base_url, url).origin + origin unless origin == request.base_url + end.uniq + end + + def script_hashes(*scripts) + scripts.map { |script| "'sha256-#{Digest::SHA256.base64digest(script)}'" } + end +end diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index 54f47cf..e343526 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -1,25 +1,18 @@ -# Be sure to restart your server when you modify this file. - -# Define an application-wide content security policy. -# See the Securing Rails Applications Guide for more information: -# https://guides.rubyonrails.org/security.html#content-security-policy-header - -# 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 -# # Specify URI for violation reports -# # policy.report_uri "/csp-violation-report-endpoint" -# end +# Enforcing baseline policy. Scripts run only from this origin; the two inline +# scripts the importmap helper emits are allowed by hash (InlineScriptHashes), +# so there is no per-request nonce and rendered HTML stays cacheable. # -# # Generate session nonces for permitted importmap and inline scripts -# config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s } -# config.content_security_policy_nonce_directives = %w(script-src) +# Nothing else is restricted: images, media, fonts, styles (including an +# account's custom styles) and connections behave as before. # -# # Report violations without enforcing the policy. -# # config.content_security_policy_report_only = true -# end +# DISABLE_CONTENT_SECURITY_POLICY=true is the escape hatch for an install whose +# proxy or customization injects scripts the policy would block. +unless ENV["DISABLE_CONTENT_SECURITY_POLICY"] == "true" + Rails.application.configure do + config.content_security_policy do |policy| + policy.script_src :self + policy.object_src :none + policy.base_uri :self + end + end +end diff --git a/test/controllers/cached_responses_test.rb b/test/controllers/cached_responses_test.rb index 8968aa0..fa420f4 100644 --- a/test/controllers/cached_responses_test.rb +++ b/test/controllers/cached_responses_test.rb @@ -26,6 +26,18 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest ActionController::Base.perform_caching = @previous_caching end + test "a reused page carries the same enforcing policy, with no nonce" do + get room_messages_url(@room) + first = response.headers["Content-Security-Policy"] + assert_match "script-src 'self' 'sha256-", first + assert_no_match "nonce-", first + + MessagesController.any_instance.expects(:find_paged_messages).never + get room_messages_url(@room) + assert_response :success + assert_equal first, response.headers["Content-Security-Policy"] + end + test "all four read actions reuse completed pages" do urls = [ room_url(@room), room_messages_url(@room), user_sidebar_url(:me), searches_url(q: "hello") ] urls.each do |url| diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb new file mode 100644 index 0000000..7e78d38 --- /dev/null +++ b/test/integration/content_security_policy_test.rb @@ -0,0 +1,78 @@ +require "test_helper" + +class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest + PAGE_PATH = "/" + + setup { sign_in :david } + + test "enforcing policy allows exactly the inline scripts the page carries" do + get root_url + follow_redirect! while response.redirect? + + policy = response.headers["Content-Security-Policy"] + assert policy, "expected an enforcing Content-Security-Policy header" + assert_nil response.headers["Content-Security-Policy-Report-Only"] + assert_match "object-src 'none'", policy + assert_match "base-uri 'self'", policy + assert_no_match "unsafe-inline", policy + assert_no_match "nonce-", policy + + script_src = policy[/script-src ([^;]+)/, 1].split + inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" } + assert_operator inline.size, :>=, 2 + inline.each do |script| + assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(script.text)}'", "inline #{script["type"]} script not allowed by hash" + end + end + + test "scripts may load from a configured asset host, however it's written" do + { + "https://cdn.example.test" => %r{\Ahttps://cdn\.example\.test\z}, + "cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z}, + "//cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z}, + "https://assets%d.example.test" => %r{\Ahttps://assets[0-3]\.example\.test\z}, + ->(source) { "https://one.example.test" } => %r{\Ahttps://one\.example\.test\z}, + ->(source, request) { "https://#{request.host}.example.test" } => %r{\Ahttps://www\.example\.com\.example\.test\z} + }.each do |asset_host, expected_origin| + with_asset_host(asset_host) do + get PAGE_PATH + follow_redirect! while response.redirect? + assert_response :success + + script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split + page = Nokogiri::HTML5(response.body) + urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values + + page.css("link[rel=modulepreload]").map { it["href"] } + page.css("script[src]").map { it["src"] } + origins = urls.map { URI.join(request.base_url, it).origin }.uniq - [ request.base_url ] + + assert_not_empty origins, asset_host.inspect + origins.each do |origin| + assert_match expected_origin, origin, asset_host.inspect + assert_includes script_src, origin, asset_host.inspect + end + assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(page.at_css("script[type=importmap]").text)}'", asset_host.inspect + end + end + end + + test "the escape hatch boots without a policy" do + output = IO.popen({ "DISABLE_CONTENT_SECURITY_POLICY" => "true" }, + [ "bin/rails", "runner", "print Rails.application.config.content_security_policy.inspect" ], chdir: Rails.root, err: File::NULL, &:read) + + assert_equal "nil", output + end + + test "uploaded files are guarded on their way out" do + assert_includes Rails.application.middleware.map(&:klass), UploadedScriptGuard + end + + private + def with_asset_host(asset_host) + previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host + Rails.application.importmap.send(:clear_cache) + yield + ensure + ApplicationController.asset_host = previous + Rails.application.importmap.send(:clear_cache) + end +end From d713f2631db9b41d039ad20bd3744626d4591df6 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 17:07:38 -0700 Subject: [PATCH 3/8] Say how long a pre-upgrade cached upload can last Disk URLs are cached for an hour, but blob proxy URLs are cached publicly and as immutable for a hundred years, so a copy taken that way before the upgrade doesn't expire. --- config/initializers/uploaded_script_guard.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb index 18cf59e..1ce8550 100644 --- a/config/initializers/uploaded_script_guard.rb +++ b/config/initializers/uploaded_script_guard.rb @@ -6,10 +6,13 @@ # a download, whatever its URL says. Only reads are touched: uploading # (direct upload JSON, Disk PUT) answers in its own types. # -# Accepted on upgrade: a browser that cached an upload under a script type -# before this guard shipped can still run it from that cache until the entry -# expires, up to an hour after it was fetched (config/initializers/active_storage.rb). -# Revalidation doesn't help, since a 304 carries no content type. +# Accepted on upgrade: a cache that stored an upload under a script type +# before this guard shipped can still serve it as one until the entry expires. +# For Disk URLs that's up to an hour (config/initializers/active_storage.rb). +# Blob proxy URLs are cached publicly and as immutable for a hundred years +# (Rails' http_cache_forever), so a copy a browser or a shared cache took that +# way doesn't expire. Revalidation doesn't help, since a 304 carries no +# content type. class UploadedScriptGuard SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain)\z} From 9c37f41ff6790260edc91ea662b4cee91ebede68 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 17:45:56 -0700 Subject: [PATCH 4/8] Keep a multi-range upload response's multipart type A request for several byte ranges gets a 206 multipart/byteranges whose boundary the client needs to split the body. Rewriting it to octet-stream lost the boundary. No browser runs that type as a script, so it passes through. --- config/initializers/uploaded_script_guard.rb | 6 ++++-- test/middleware/uploaded_script_guard_test.rb | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb index 1ce8550..97eb00c 100644 --- a/config/initializers/uploaded_script_guard.rb +++ b/config/initializers/uploaded_script_guard.rb @@ -3,7 +3,9 @@ # Browsers accept sixteen spellings of that type, and Disk URLs sign the type # they were minted with, so a list of types to refuse can't cover old links. # Instead, anything Active Storage serves that isn't plainly media goes out as -# a download, whatever its URL says. Only reads are touched: uploading +# a download, whatever its URL says. A multi-range response keeps its +# multipart/byteranges type and boundary, which no browser runs as a script. +# Only reads are touched: uploading # (direct upload JSON, Disk PUT) answers in its own types. # # Accepted on upgrade: a cache that stored an upload under a script type @@ -14,7 +16,7 @@ # way doesn't expire. Revalidation doesn't help, since a 304 carries no # content type. class UploadedScriptGuard - SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain)\z} + SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain|multipart/byteranges)\z} def initialize(app) @app = app diff --git a/test/middleware/uploaded_script_guard_test.rb b/test/middleware/uploaded_script_guard_test.rb index c627c9f..7663c3c 100644 --- a/test/middleware/uploaded_script_guard_test.rb +++ b/test/middleware/uploaded_script_guard_test.rb @@ -21,6 +21,11 @@ class UploadedScriptGuardTest < ActiveSupport::TestCase end end + test "multi-range responses keep their boundary" do + type = "multipart/byteranges; boundary=AaB03x" + assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/blobs/proxy/key/movie.mp4") + end + test "direct upload responses keep their JSON type" do assert_equal "application/json", served_type("application/json", "#{ActiveStorage.routes_prefix}/direct_uploads", method: "POST") end From 2a449401ec4e129519e66d59771240db967b84c6 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 18:04:05 -0700 Subject: [PATCH 5/8] Leave a request without a policy when the policy is turned off With DISABLE_CONTENT_SECURITY_POLICY=true there's no application policy, and the hash callback started an empty one, so pages sent an empty Content-Security-Policy header. Skip the callback then. --- .../concerns/inline_script_hashes.rb | 4 +++- .../content_security_policy_test.rb | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb index c0fde41..bf2761e 100644 --- a/app/controllers/concerns/inline_script_hashes.rb +++ b/app/controllers/concerns/inline_script_hashes.rb @@ -8,7 +8,9 @@ module InlineScriptHashes ENTRY_POINT_SCRIPT = %(import "application") included do - content_security_policy do |policy| + # With the policy turned off there's nothing to add to, so leave the + # request without one rather than starting an empty policy + content_security_policy if: -> { request.content_security_policy } do |policy| if sources = policy.directives["script-src"] importmap = Rails.application.importmap.to_json(resolver: helpers) policy.directives["script-src"] = sources | script_origins(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT) diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb index 7e78d38..3f0eac7 100644 --- a/test/integration/content_security_policy_test.rb +++ b/test/integration/content_security_policy_test.rb @@ -55,6 +55,16 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest end end + test "with the policy turned off, pages send none" do + without_policy do + get PAGE_PATH + follow_redirect! while response.redirect? + + assert_response :success + assert_nil response.headers["Content-Security-Policy"] + end + end + test "the escape hatch boots without a policy" do output = IO.popen({ "DISABLE_CONTENT_SECURITY_POLICY" => "true" }, [ "bin/rails", "runner", "print Rails.application.config.content_security_policy.inspect" ], chdir: Rails.root, err: File::NULL, &:read) @@ -67,6 +77,14 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest end private + # What DISABLE_CONTENT_SECURITY_POLICY=true leaves at boot: no application policy + def without_policy + previous = Rails.application.env_config.delete("action_dispatch.content_security_policy") + yield + ensure + Rails.application.env_config["action_dispatch.content_security_policy"] = previous + end + def with_asset_host(asset_host) previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host Rails.application.importmap.send(:clear_cache) From 2be2227f40dbe31cb0a278cb2d9d31ab2c7ca51c Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 18:42:45 -0700 Subject: [PATCH 6/8] Allow scripts only from the asset directory, not the whole origin Uploads are served from this origin too, so script-src 'self' would run one that a cache still holds under a script type from before the upload guard. Application pages now allow each importmap URL up to its asset directory (relative root and asset host included), the inline importmap by hash, and the service worker by worker-src. --- .../concerns/inline_script_hashes.rb | 31 +++++---- .../initializers/content_security_policy.rb | 9 ++- config/initializers/uploaded_script_guard.rb | 28 ++++---- test/controllers/cached_responses_test.rb | 2 +- .../content_security_policy_test.rb | 65 +++++++++++++++++-- 5 files changed, 101 insertions(+), 34 deletions(-) diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb index bf2761e..efc7fde 100644 --- a/app/controllers/concerns/inline_script_hashes.rb +++ b/app/controllers/concerns/inline_script_hashes.rb @@ -1,7 +1,8 @@ -# Allows the scripts the importmap helper emits: the inline importmap and its -# entry-point module by content hash, and the origins its URLs point to. The -# hashes change only when the importmap does (a deploy), so cached HTML stays -# valid, unlike a per-request nonce. +# Allows the scripts the importmap helper emits and nothing else: the inline +# importmap and its entry-point module by content hash, and the asset +# directory its URLs point into. Not the whole origin, because uploads are +# served from this origin too. The hashes change only when the importmap does +# (a deploy), so cached HTML stays valid, unlike a per-request nonce. module InlineScriptHashes extend ActiveSupport::Concern @@ -13,19 +14,27 @@ module InlineScriptHashes content_security_policy if: -> { request.content_security_policy } do |policy| if sources = policy.directives["script-src"] importmap = Rails.application.importmap.to_json(resolver: helpers) - policy.directives["script-src"] = sources | script_origins(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT) + policy.directives["script-src"] = (sources - [ "'self'" ]) | script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT) + policy.worker_src service_worker_url(format: :js) end end end private # The importmap is resolved by Rails' asset helpers, so its URLs already - # carry any configured asset host, however it's written: with or without a - # scheme, protocol-relative, a %d set, or a proc. - def script_origins(importmap) - JSON.parse(importmap)["imports"].values.filter_map do |url| - origin = URI.join(request.base_url, url).origin - origin unless origin == request.base_url + # carry any relative root and asset host, however the host is written: + # with or without a scheme, protocol-relative, a %d set, or a proc. Each + # is allowed up to its asset directory; one outside it is allowed exactly. + # + # A path only limits the first request: browsers check just the origin + # after a redirect. Nothing under the asset directory redirects. + def script_locations(importmap) + assets = File.join(config.relative_url_root.to_s, Rails.application.config.assets.prefix, "") + + JSON.parse(importmap)["imports"].values.map do |url| + url = URI.join(request.base_url, url) + directory = url.path[0, url.path.index(assets) + assets.length] if url.path.include?(assets) + "#{url.origin}#{directory || url.path}" end.uniq end diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index e343526..a99e648 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -1,6 +1,9 @@ -# Enforcing baseline policy. Scripts run only from this origin; the two inline -# scripts the importmap helper emits are allowed by hash (InlineScriptHashes), -# so there is no per-request nonce and rendered HTML stays cacheable. +# Enforcing baseline policy. Application pages narrow script-src from this +# origin to its asset directory, since uploads are served from this origin +# too, and allow the two inline scripts the importmap helper emits by hash +# (InlineScriptHashes). There is no per-request nonce, so rendered HTML stays +# cacheable. 'self' remains for responses outside ApplicationController, none +# of which render user content. # # Nothing else is restricted: images, media, fonts, styles (including an # account's custom styles) and connections behave as before. diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb index 97eb00c..d0badc3 100644 --- a/config/initializers/uploaded_script_guard.rb +++ b/config/initializers/uploaded_script_guard.rb @@ -1,20 +1,18 @@ -# Uploaded files are served from this origin, so under script-src 'self' an -# upload served with any JavaScript MIME type would run as one of our scripts. -# Browsers accept sixteen spellings of that type, and Disk URLs sign the type -# they were minted with, so a list of types to refuse can't cover old links. -# Instead, anything Active Storage serves that isn't plainly media goes out as -# a download, whatever its URL says. A multi-range response keeps its +# Uploaded files are served from this origin. Application pages only allow +# scripts from the asset directory (InlineScriptHashes), so an upload can't +# load as a script whatever type it's served with. This guard is the second +# layer: browsers accept sixteen spellings of a JavaScript type, and Disk URLs +# sign the type they were minted with, so instead of refusing a list of types, +# anything Active Storage serves that isn't plainly media goes out as a +# download, whatever its URL says. A multi-range response keeps its # multipart/byteranges type and boundary, which no browser runs as a script. -# Only reads are touched: uploading -# (direct upload JSON, Disk PUT) answers in its own types. +# Only reads are touched: uploading (direct upload JSON, Disk PUT) answers in +# its own types. # -# Accepted on upgrade: a cache that stored an upload under a script type -# before this guard shipped can still serve it as one until the entry expires. -# For Disk URLs that's up to an hour (config/initializers/active_storage.rb). -# Blob proxy URLs are cached publicly and as immutable for a hundred years -# (Rails' http_cache_forever), so a copy a browser or a shared cache took that -# way doesn't expire. Revalidation doesn't help, since a 304 carries no -# content type. +# A cache that stored an upload under a script type before this shipped can +# still serve it that way until the entry expires (an hour for Disk URLs, and +# never for blob proxy URLs, which Rails caches as immutable). The policy's +# path restriction is what keeps such a copy from running. class UploadedScriptGuard SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain|multipart/byteranges)\z} diff --git a/test/controllers/cached_responses_test.rb b/test/controllers/cached_responses_test.rb index fa420f4..26efc07 100644 --- a/test/controllers/cached_responses_test.rb +++ b/test/controllers/cached_responses_test.rb @@ -29,7 +29,7 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest test "a reused page carries the same enforcing policy, with no nonce" do get room_messages_url(@room) first = response.headers["Content-Security-Policy"] - assert_match "script-src 'self' 'sha256-", first + assert_match %r{script-src http://[^/ ]+/assets/ 'sha256-}, first assert_no_match "nonce-", first MessagesController.any_instance.expects(:find_paged_messages).never diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb index 3f0eac7..67cf66c 100644 --- a/test/integration/content_security_policy_test.rb +++ b/test/integration/content_security_policy_test.rb @@ -16,8 +16,11 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest assert_match "base-uri 'self'", policy assert_no_match "unsafe-inline", policy assert_no_match "nonce-", policy + assert_equal "http://www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1] script_src = policy[/script-src ([^;]+)/, 1].split + assert_not_includes script_src, "'self'" + assert_includes script_src, "http://www.example.com/assets/" inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" } assert_operator inline.size, :>=, 2 inline.each do |script| @@ -46,15 +49,43 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest origins = urls.map { URI.join(request.base_url, it).origin }.uniq - [ request.base_url ] assert_not_empty origins, asset_host.inspect - origins.each do |origin| - assert_match expected_origin, origin, asset_host.inspect - assert_includes script_src, origin, asset_host.inspect - end + origins.each { assert_match expected_origin, it, asset_host.inspect } + assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }, asset_host.inspect assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(page.at_css("script[type=importmap]").text)}'", asset_host.inspect end end end + test "scripts are limited to the asset directory under a relative root" do + with_relative_url_root("/chat") do + get PAGE_PATH + follow_redirect! while response.redirect? + assert_response :success + + script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split + page = Nokogiri::HTML5(response.body) + urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values + page.css("link[rel=modulepreload]").map { it["href"] } + + assert urls.all? { it.start_with?("/chat/assets/") }, urls.inspect + assert_includes script_src, "http://www.example.com/chat/assets/" + assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s } + end + end + + test "uploads can't load as scripts" do + get PAGE_PATH + follow_redirect! while response.redirect? + + script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split + blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new("alert(1)"), filename: "upload.js", content_type: "text/javascript") + + disk_url = ActiveStorage::Current.set(url_options: { protocol: "http", host: "www.example.com" }) { blob.url } + + [ rails_blob_url(blob), rails_blob_url(blob, disposition: :inline), rails_storage_proxy_url(blob), disk_url ].each do |url| + assert_not allowed_by_path?(script_src, url), "#{url} allowed by #{script_src.inspect}" + end + end + test "with the policy turned off, pages send none" do without_policy do get PAGE_PATH @@ -85,6 +116,32 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest Rails.application.env_config["action_dispatch.content_security_policy"] = previous end + # Every page script is allowed by a source with a path, never by a bare origin + def assert_scripts_allowed_by_path(script_src, urls, message = nil) + assert_empty script_src.grep(%r{\A[a-z]+://[^/]+/?\z}), message + urls.each { assert allowed_by_path?(script_src, it), "#{it} not allowed by #{script_src.inspect} #{message}" } + end + + # CSP source matching on a first request: 'self' and a bare origin match any + # path, a path ending in / matches as a prefix, any other path exactly + def allowed_by_path?(script_src, url) + url = URI(url) + script_src.any? do |source| + source = URI(source == "'self'" ? request.base_url : source) if source == "'self'" || source.match?(%r{\A[a-z]+://}) + source.is_a?(URI) && source.origin == url.origin && + (source.path.empty? || (source.path.end_with?("/") ? url.path.start_with?(source.path) : url.path == source.path)) + end + end + + def with_relative_url_root(root) + previous, ApplicationController.config.relative_url_root = ApplicationController.config.relative_url_root, root + Rails.application.importmap.send(:clear_cache) + yield + ensure + ApplicationController.config.relative_url_root = previous + Rails.application.importmap.send(:clear_cache) + end + def with_asset_host(asset_host) previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host Rails.application.importmap.send(:clear_cache) From 0a494a10cbcdd065805c06fcc984aaf966375959 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 18:59:39 -0700 Subject: [PATCH 7/8] Leave the scheme off this origin's script sources Behind a proxy that terminates TLS without X-Forwarded-Proto, Rails sees http while the page is https, and an http source on a non-default port doesn't match. A source without a scheme takes the page's own. --- .../concerns/inline_script_hashes.rb | 8 ++++-- test/controllers/cached_responses_test.rb | 2 +- .../content_security_policy_test.rb | 25 ++++++++++++++----- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb index efc7fde..357a70d 100644 --- a/app/controllers/concerns/inline_script_hashes.rb +++ b/app/controllers/concerns/inline_script_hashes.rb @@ -15,7 +15,7 @@ module InlineScriptHashes if sources = policy.directives["script-src"] importmap = Rails.application.importmap.to_json(resolver: helpers) policy.directives["script-src"] = (sources - [ "'self'" ]) | script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT) - policy.worker_src service_worker_url(format: :js) + policy.worker_src "#{request.host_with_port}#{service_worker_path(format: :js)}" end end end @@ -26,6 +26,9 @@ module InlineScriptHashes # with or without a scheme, protocol-relative, a %d set, or a proc. Each # is allowed up to its asset directory; one outside it is allowed exactly. # + # This origin's own sources leave out the scheme, so they match the page's + # scheme even behind a proxy that terminates TLS without saying so. + # # A path only limits the first request: browsers check just the origin # after a redirect. Nothing under the asset directory redirects. def script_locations(importmap) @@ -33,8 +36,9 @@ module InlineScriptHashes JSON.parse(importmap)["imports"].values.map do |url| url = URI.join(request.base_url, url) + origin = url.origin == request.base_url ? request.host_with_port : url.origin directory = url.path[0, url.path.index(assets) + assets.length] if url.path.include?(assets) - "#{url.origin}#{directory || url.path}" + "#{origin}#{directory || url.path}" end.uniq end diff --git a/test/controllers/cached_responses_test.rb b/test/controllers/cached_responses_test.rb index 26efc07..888994d 100644 --- a/test/controllers/cached_responses_test.rb +++ b/test/controllers/cached_responses_test.rb @@ -29,7 +29,7 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest test "a reused page carries the same enforcing policy, with no nonce" do get room_messages_url(@room) first = response.headers["Content-Security-Policy"] - assert_match %r{script-src http://[^/ ]+/assets/ 'sha256-}, first + assert_match %r{script-src [^/ ]+/assets/ 'sha256-}, first assert_no_match "nonce-", first MessagesController.any_instance.expects(:find_paged_messages).never diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb index 67cf66c..b4672e0 100644 --- a/test/integration/content_security_policy_test.rb +++ b/test/integration/content_security_policy_test.rb @@ -16,11 +16,11 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest assert_match "base-uri 'self'", policy assert_no_match "unsafe-inline", policy assert_no_match "nonce-", policy - assert_equal "http://www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1] + assert_equal "www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1] script_src = policy[/script-src ([^;]+)/, 1].split assert_not_includes script_src, "'self'" - assert_includes script_src, "http://www.example.com/assets/" + assert_includes script_src, "www.example.com/assets/" inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" } assert_operator inline.size, :>=, 2 inline.each do |script| @@ -67,11 +67,21 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values + page.css("link[rel=modulepreload]").map { it["href"] } assert urls.all? { it.start_with?("/chat/assets/") }, urls.inspect - assert_includes script_src, "http://www.example.com/chat/assets/" + assert_includes script_src, "www.example.com/chat/assets/" assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s } end end + test "this origin's sources keep a non-default port and take the page's scheme" do + host! "www.example.com:8443" + get PAGE_PATH + follow_redirect! while response.redirect? + + policy = response.headers["Content-Security-Policy"] + assert_includes policy[/script-src ([^;]+)/, 1].split, "www.example.com:8443/assets/" + assert_equal "www.example.com:8443/service-worker.js", policy[/worker-src ([^;]+)/, 1] + end + test "uploads can't load as scripts" do get PAGE_PATH follow_redirect! while response.redirect? @@ -118,16 +128,19 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest # Every page script is allowed by a source with a path, never by a bare origin def assert_scripts_allowed_by_path(script_src, urls, message = nil) - assert_empty script_src.grep(%r{\A[a-z]+://[^/]+/?\z}), message + assert_empty script_src.grep(%r{\A([a-z]+://)?[^/']+/?\z}), message urls.each { assert allowed_by_path?(script_src, it), "#{it} not allowed by #{script_src.inspect} #{message}" } end # CSP source matching on a first request: 'self' and a bare origin match any - # path, a path ending in / matches as a prefix, any other path exactly + # path, a path ending in / matches as a prefix, any other path exactly. A + # source without a scheme takes the page's. def allowed_by_path?(script_src, url) url = URI(url) script_src.any? do |source| - source = URI(source == "'self'" ? request.base_url : source) if source == "'self'" || source.match?(%r{\A[a-z]+://}) + unless source.start_with?("'") && source != "'self'" + source = URI(source == "'self'" ? request.base_url : (source.include?("://") ? source : "#{request.scheme}://#{source}")) + end source.is_a?(URI) && source.origin == url.origin && (source.path.empty? || (source.path.end_with?("/") ? url.path.start_with?(source.path) : url.path == source.path)) end From 2ba940c683b5c193d0ec0b3bb2ab3b4f73b60330 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 19:13:26 -0700 Subject: [PATCH 8/8] Allow an asset host only up to its own asset directory An asset host's path can contain the asset directory too, as in https://cdn.example.com/assets, and cutting each URL at the first one would allow everything the host serves under it. Cut at the last one instead. --- .../concerns/inline_script_hashes.rb | 6 ++++-- .../content_security_policy_test.rb | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb index 357a70d..4577292 100644 --- a/app/controllers/concerns/inline_script_hashes.rb +++ b/app/controllers/concerns/inline_script_hashes.rb @@ -24,7 +24,9 @@ module InlineScriptHashes # The importmap is resolved by Rails' asset helpers, so its URLs already # carry any relative root and asset host, however the host is written: # with or without a scheme, protocol-relative, a %d set, or a proc. Each - # is allowed up to its asset directory; one outside it is allowed exactly. + # is allowed up to its asset directory, the last one in its path, since an + # asset host's own path may contain one too; a URL outside it is allowed + # exactly. # # This origin's own sources leave out the scheme, so they match the page's # scheme even behind a proxy that terminates TLS without saying so. @@ -37,7 +39,7 @@ module InlineScriptHashes JSON.parse(importmap)["imports"].values.map do |url| url = URI.join(request.base_url, url) origin = url.origin == request.base_url ? request.host_with_port : url.origin - directory = url.path[0, url.path.index(assets) + assets.length] if url.path.include?(assets) + directory = url.path[0, url.path.rindex(assets) + assets.length] if url.path.include?(assets) "#{origin}#{directory || url.path}" end.uniq end diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb index b4672e0..7a3aaf6 100644 --- a/test/integration/content_security_policy_test.rb +++ b/test/integration/content_security_policy_test.rb @@ -56,6 +56,27 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest end end + test "an asset host whose path contains the asset directory is limited to the directory it serves" do + with_asset_host("https://cdn.example.test/assets") do + get PAGE_PATH + follow_redirect! while response.redirect? + + script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split + assert_includes script_src, "https://cdn.example.test/assets/assets/" + assert_not allowed_by_path?(script_src, "https://cdn.example.test/assets/rails/active_storage/blobs/proxy/x/upload.js") + end + end + + test "the sign-in page is limited to the asset directory too" do + cookies.delete(:session_token) + get new_session_url + assert_response :success + + script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split + assert_not_includes script_src, "'self'" + assert_includes script_src, "www.example.com/assets/" + end + test "scripts are limited to the asset directory under a relative root" do with_relative_url_root("/chat") do get PAGE_PATH