Don't set upgrade-insecure-requests directive for HTTP requests

lib/secure_headers.rb

@@ -133,6 +133,7 @@ def opt_out_of_all_protection(request)
     # request.
     #
     # StrictTransportSecurity is not applied to http requests.
+    # upgrade_insecure_requests is not applied to http requests.
     # See #config_for to determine which config is used for a given request.
     #
     # Returns a hash of header names => header values. The value
@@ -146,6 +147,11 @@ def header_hash_for(request)
 
       if request.scheme != HTTPS
         headers.delete(StrictTransportSecurity::HEADER_NAME)
+
+        # Remove upgrade_insecure_requests from CSP headers for HTTP requests
+        # as it doesn't make sense to upgrade requests when the page itself is served over HTTP
+        remove_upgrade_insecure_requests_from_csp!(headers, config.csp)
+        remove_upgrade_insecure_requests_from_csp!(headers, config.csp_report_only)
       end
       headers
     end
@@ -242,6 +248,23 @@ def content_security_policy_nonce(request, script_or_style)
     def override_secure_headers_request_config(request, config)
       request.env[SECURE_HEADERS_CONFIG] = config
     end
+
+    # Private: removes upgrade_insecure_requests directive from a CSP config
+    # if it's present, and updates the headers hash with the modified CSP.
+    #
+    # headers - the headers hash to update
+    # csp_config - the CSP config to check and potentially modify
+    #
+    # Returns nothing (modifies headers in place)
+    def remove_upgrade_insecure_requests_from_csp!(headers, csp_config)
+      return if csp_config.opt_out?
+      return unless csp_config.directive_value(ContentSecurityPolicy::UPGRADE_INSECURE_REQUESTS)
+
+      modified_config = csp_config.dup
+      modified_config.update_directive(ContentSecurityPolicy::UPGRADE_INSECURE_REQUESTS, false)
+      header_name, value = ContentSecurityPolicy.make_header(modified_config)
+      headers[header_name] = value if header_name && value
+    end
   end
 
   # These methods are mixed into controllers and delegate to the class method

spec/lib/secure_headers_spec.rb

@@ -436,6 +436,68 @@ module SecureHeaders
 
           end
         end
+
+        it "does not set upgrade-insecure-requests if request is over HTTP" do
+          reset_config
+          Configuration.default do |config|
+            config.csp = {
+              default_src: %w('self'),
+              script_src: %w('self'),
+              upgrade_insecure_requests: true
+            }
+          end
+
+          plaintext_request = Rack::Request.new({})
+          hash = SecureHeaders.header_hash_for(plaintext_request)
+          expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'")
+          expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).not_to include("upgrade-insecure-requests")
+        end
+
+        it "sets upgrade-insecure-requests if request is over HTTPS" do
+          reset_config
+          Configuration.default do |config|
+            config.csp = {
+              default_src: %w('self'),
+              script_src: %w('self'),
+              upgrade_insecure_requests: true
+            }
+          end
+
+          https_request = Rack::Request.new("HTTPS" => "on")
+          hash = SecureHeaders.header_hash_for(https_request)
+          expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'; upgrade-insecure-requests")
+        end
+
+        it "does not set upgrade-insecure-requests in report-only mode if request is over HTTP" do
+          reset_config
+          Configuration.default do |config|
+            config.csp_report_only = {
+              default_src: %w('self'),
+              script_src: %w('self'),
+              upgrade_insecure_requests: true
+            }
+          end
+
+          plaintext_request = Rack::Request.new({})
+          hash = SecureHeaders.header_hash_for(plaintext_request)
+          expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'")
+          expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).not_to include("upgrade-insecure-requests")
+        end
+
+        it "sets upgrade-insecure-requests in report-only mode if request is over HTTPS" do
+          reset_config
+          Configuration.default do |config|
+            config.csp_report_only = {
+              default_src: %w('self'),
+              script_src: %w('self'),
+              upgrade_insecure_requests: true
+            }
+          end
+
+          https_request = Rack::Request.new("HTTPS" => "on")
+          hash = SecureHeaders.header_hash_for(https_request)
+          expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'; upgrade-insecure-requests")
+        end
       end
     end