Merge pull request #312 from rails/rm-opt-in-integrity

rafaelfranca · web-flow · commit b51f709119a9 · 2025-07-31T15:05:00.000-04:00
Make integrity calculation opt-in
diff --git a/README.md b/README.md
@@ -81,12 +81,13 @@ If you want to import local js module files from `app/javascript/src` or other s
 pin_all_from 'app/javascript/src', under: 'src', to: 'src'
 
 # With automatic integrity calculation for enhanced security
+enable_integrity!
 pin_all_from 'app/javascript/controllers', under: 'controllers', integrity: true
 ```
 
 The `:to` parameter is only required if you want to change the destination logical import name. If you drop the :to option, you must place the :under option directly after the first parameter.
 
-The `integrity: true` option automatically calculates integrity hashes for all files in the directory, providing security benefits without manual hash management.
+The `enable_integrity!` call enables integrity calculation globally, and `integrity: true` automatically calculates integrity hashes for all files in the directory, providing security benefits without manual hash management.
 
 Allows you to:
 
@@ -142,12 +143,15 @@ For enhanced security, importmap-rails supports [Subresource Integrity (SRI)](ht
 
 ### Automatic integrity for local assets
 
-Starting with importmap-rails, **`integrity: true` is the default** for all pins. This automatically calculates integrity hashes for local assets served by the Rails asset pipeline:
+To enable automatic integrity calculation for local assets served by the Rails asset pipeline, you must first call `enable_integrity!` in your importmap configuration:
 
 ```ruby
 # config/importmap.rb
 
-# These all use integrity: true by default
+# Enable integrity calculation globally
+enable_integrity!
+
+# With integrity enabled, these will auto-calculate integrity hashes
 pin "application"                                               # Auto-calculated integrity
 pin "admin", to: "admin.js"                                     # Auto-calculated integrity
 pin_all_from "app/javascript/controllers", under: "controllers" # Auto-calculated integrity
@@ -163,7 +167,7 @@ This is particularly useful for:
 * **Bulk operations** with `pin_all_from` where calculating hashes manually would be tedious
 * **Development workflow** where asset contents change frequently
 
-This behavior can be disabled by setting `integrity: false` or `integrity: nil`
+**Note:** Integrity calculation is opt-in and must be enabled with `enable_integrity!`. This behavior can be further controlled by setting `integrity: false` or `integrity: nil` on individual pins.
 
 **Important for Propshaft users:** SRI support requires Propshaft 1.2+ and you must configure the integrity hash algorithm in your application:
 
@@ -174,7 +178,7 @@ config.assets.integrity_hash_algorithm = 'sha256'  # or 'sha384', 'sha512'
 
 Without this configuration, integrity will be disabled by default when using Propshaft. Sprockets includes integrity support out of the box.
 
-**Example output with `integrity: true`:**
+**Example output with `enable_integrity!` and `integrity: true`:**
 ```json
 {
   "imports": {
diff --git a/lib/importmap/map.rb b/lib/importmap/map.rb
@@ -12,6 +12,7 @@ def self.pin_line_regexp_for(package) # :nodoc:
   class InvalidFile < StandardError; end
 
   def initialize
+    @integrity = false
     @packages, @directories = {}, {}
     @cache = {}
   end
@@ -31,6 +32,43 @@ def draw(path = nil, &block)
     self
   end
 
+  # Enables automatic integrity hash calculation for all pinned modules.
+  #
+  # When enabled, integrity values are included in the importmap JSON for all
+  # pinned modules. For local assets served by the Rails asset pipeline,
+  # integrity hashes are automatically calculated when +integrity: true+ is
+  # specified. For modules with explicit integrity values, those values are
+  # included as provided. This provides Subresource Integrity (SRI) protection
+  # to ensure JavaScript modules haven't been tampered with.
+  #
+  # Clears the importmap cache when called to ensure fresh integrity hashes
+  # are generated.
+  #
+  # ==== Examples
+  #
+  #   # config/importmap.rb
+  #   enable_integrity!
+  #
+  #   # These will now auto-calculate integrity hashes
+  #   pin "application"                   # integrity: true by default
+  #   pin "admin", to: "admin.js"         # integrity: true by default
+  #   pin_all_from "app/javascript/lib"   # integrity: true by default
+  #
+  #   # Manual control still works
+  #   pin "no_integrity", integrity: false
+  #   pin "custom_hash", integrity: "sha384-abc123..."
+  #
+  # ==== Notes
+  #
+  # * Integrity calculation is disabled by default and must be explicitly enabled
+  # * Requires asset pipeline support for integrity calculation (Sprockets or Propshaft 1.2+)
+  # * For Propshaft, you must configure +config.assets.integrity_hash_algorithm+
+  # * External CDN packages should provide their own integrity hashes
+  def enable_integrity!
+    clear_cache
+    @integrity = true
+  end
+
   def pin(name, to: nil, preload: true, integrity: true)
     clear_cache
     @packages[name] = MappedFile.new(name: name, path: to || "#{name}.js", preload: preload, integrity: integrity)
@@ -216,6 +254,8 @@ def build_integrity_hash(packages, resolver:)
     end
 
     def resolve_integrity_value(integrity, path, resolver:)
+      return unless @integrity
+
       case integrity
       when true
         resolver.asset_integrity(path) if resolver.respond_to?(:asset_integrity)
diff --git a/test/dummy/config/importmap.rb b/test/dummy/config/importmap.rb
@@ -1,3 +1,5 @@
+enable_integrity!
+
 pin_all_from "app/assets/javascripts"
 
 pin "md5", to: "https://cdn.skypack.dev/md5", preload: true, integrity: false
diff --git a/test/importmap_test.rb b/test/importmap_test.rb
@@ -5,6 +5,8 @@ class ImportmapTest < ActiveSupport::TestCase
   def setup
     @importmap = Importmap::Map.new.tap do |map|
       map.draw do
+        enable_integrity!
+
         pin "application", preload: false, integrity: false
         pin "editor", to: "rich_text.js", preload: false, integrity: "sha384-OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb"
         pin "not_there", to: "nowhere.js", preload: false, integrity: "sha384-somefakehash"
@@ -39,11 +41,21 @@ def setup
     assert_not_includes generate_importmap_json["integrity"].values, "sha384-somefakehash"
   end
 
-  test "integrity is default" do
+  test "integrity is not on by default" do
     @importmap = Importmap::Map.new.tap do |map|
       map.pin "application", preload: false
     end
 
+    json = generate_importmap_json
+    assert_not json.key?("integrity")
+  end
+
+    test "enable_integrity! change the map to generate integrity attribute" do
+    @importmap = Importmap::Map.new.tap do |map|
+      map.enable_integrity!
+      map.pin "application", preload: false
+    end
+
     json = generate_importmap_json
     assert json.key?("integrity")
     application_path = json["imports"]["application"]
@@ -68,7 +80,9 @@ def setup
 
   test "integrity: 'custom-hash' uses the provided string" do
     custom_hash = "sha384-customhash123"
+
     @importmap = Importmap::Map.new.tap do |map|
+      map.enable_integrity!
       map.pin "application", preload: false, integrity: custom_hash
     end
 
@@ -122,6 +136,7 @@ def setup
 
   test "importmap json includes integrity hashes from integrity: true" do
     importmap = Importmap::Map.new.tap do |map|
+      map.enable_integrity!
       map.pin "application", integrity: true
     end
 
@@ -270,13 +285,11 @@ def setup
     assert_equal "https://cdn.skypack.dev/tinymce", tinymce.path
     assert_equal 'alternate', tinymce.preload
 
-    # Should include packages for multiple entry points (chartkick preloads for both 'application' and 'alternate')
     chartkick = packages["https://cdn.skypack.dev/chartkick"]
     assert chartkick, "Should include chartkick package"
     assert_equal "chartkick", chartkick.name
     assert_equal ['application', 'alternate'], chartkick.preload
 
-    # Should include always-preloaded packages
     md5 = packages["https://cdn.skypack.dev/md5"]
     assert md5, "Should include md5 package (always preloaded)"
 
@@ -290,15 +303,12 @@ def setup
     leaflet = packages["https://cdn.skypack.dev/leaflet"]
     assert leaflet, "Should include leaflet package for application entry point"
 
-    # Should include packages for 'alternate' entry point
     tinymce = packages["https://cdn.skypack.dev/tinymce"]
     assert tinymce, "Should include tinymce package for alternate entry point"
 
-    # Should include packages for multiple entry points
     chartkick = packages["https://cdn.skypack.dev/chartkick"]
     assert chartkick, "Should include chartkick package for both entry points"
 
-    # Should include always-preloaded packages
     md5 = packages["https://cdn.skypack.dev/md5"]
     assert md5, "Should include md5 package (always preloaded)"
 
@@ -307,8 +317,8 @@ def setup
   end
 
   test "preloaded_module_packages includes package integrity when present" do
-    # Create a new importmap with a preloaded package that has integrity
     importmap = Importmap::Map.new.tap do |map|
+      map.enable_integrity!
       map.pin "editor", to: "rich_text.js", preload: true, integrity: "sha384-OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb"
     end
 
@@ -322,6 +332,7 @@ def setup
 
   test "pin with integrity: true should calculate integrity dynamically" do
     importmap = Importmap::Map.new.tap do |map|
+      map.enable_integrity!
       map.pin "editor", to: "rich_text.js", preload: true, integrity: "sha384-OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb"
     end
 
@@ -370,6 +381,7 @@ def setup
 
   test "pin_all_from with integrity: true should calculate integrity dynamically" do
     importmap = Importmap::Map.new.tap do |map|
+      map.enable_integrity!
       map.pin_all_from "app/javascript/controllers", under: "controllers", integrity: true
     end
 
