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.

app/controllers/concerns/inline_script_hashes.rb

@@ -1,7 +1,8 @@
-# Allows the scripts the importmap helper emits: the inline importmap and its
+# Allows the scripts the importmap helper emits and nothing else: the inline
-# entry-point module by content hash, and the origins its URLs point to. The
+# importmap and its entry-point module by content hash, and the asset
-# hashes change only when the importmap does (a deploy), so cached HTML stays
+# directory its URLs point into. Not the whole origin, because uploads are
-# valid, unlike a per-request nonce.
+# 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
+    # carry any relative root and asset host, however the host is written:
-    # scheme, protocol-relative, a %d set, or a proc.
+    # with or without a scheme, protocol-relative, a %d set, or a proc. Each
-    def script_origins(importmap)
+    # is allowed up to its asset directory; one outside it is allowed exactly.
-      JSON.parse(importmap)["imports"].values.filter_map do |url|
+    #
-        origin = URI.join(request.base_url, url).origin
+    # A path only limits the first request: browsers check just the origin
-        origin unless origin == request.base_url
+    # 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
 

config/initializers/content_security_policy.rb

@@ -1,6 +1,9 @@
-# Enforcing baseline policy. Scripts run only from this origin; the two inline
+# Enforcing baseline policy. Application pages narrow script-src from this
-# scripts the importmap helper emits are allowed by hash (InlineScriptHashes),
+# origin to its asset directory, since uploads are served from this origin
-# so there is no per-request nonce and rendered HTML stays cacheable.
+# 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.

config/initializers/uploaded_script_guard.rb

@@ -1,20 +1,18 @@
-# Uploaded files are served from this origin, so under script-src 'self' an
+# Uploaded files are served from this origin. Application pages only allow
-# upload served with any JavaScript MIME type would run as one of our scripts.
+# scripts from the asset directory (InlineScriptHashes), so an upload can't
-# Browsers accept sixteen spellings of that type, and Disk URLs sign the type
+# load as a script whatever type it's served with. This guard is the second
-# they were minted with, so a list of types to refuse can't cover old links.
+# layer: browsers accept sixteen spellings of a JavaScript type, and Disk URLs
-# Instead, anything Active Storage serves that isn't plainly media goes out as
+# sign the type they were minted with, so instead of refusing a list of types,
-# a download, whatever its URL says. A multi-range response keeps its
+# 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
+# Only reads are touched: uploading (direct upload JSON, Disk PUT) answers in
-# (direct upload JSON, Disk PUT) answers in its own types.
+# its own types.
 #
-# Accepted on upgrade: a cache that stored an upload under a script type
+# A cache that stored an upload under a script type before this shipped can
-# before this guard shipped can still serve it as one until the entry expires.
+# still serve it that way until the entry expires (an hour for Disk URLs, and
-# For Disk URLs that's up to an hour (config/initializers/active_storage.rb).
+# never for blob proxy URLs, which Rails caches as immutable). The policy's
-# Blob proxy URLs are cached publicly and as immutable for a hundred years
+# path restriction is what keeps such a copy from running.
-# (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|multipart/byteranges)\z}
 

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

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|
+        origins.each { assert_match expected_origin, it, asset_host.inspect }
-          assert_match expected_origin, origin, 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, 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 "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)