refactor: Use configuration-based approach for server bundle output path

- Remove enforce_secure_server_bundles from bundle resolution logic
- Simplify utils.rb by removing hard-coded paths
- Use server_bundle_output_path configuration directly
- Add validation to ensure enforce_secure_server_bundles and server_bundle_output_path are compatible
- Set server_bundle_output_path default to 'ssr-generated'
- Update generator templates to use secure server bundle configuration
- Update tests to match new implementation

Co-authored-by: Abanoub Ghadban <AbanoubGhadban@users.noreply.github.com>

Author: github-actions[bot]

lib/generators/react_on_rails/base_generator.rb

@@ -136,14 +136,17 @@ def update_gitignore_for_auto_registration
         return unless File.exist?(gitignore_path)
 
         gitignore_content = File.read(gitignore_path)
-        return if gitignore_content.include?("**/generated/**")
 
-        append_to_file ".gitignore" do
-          <<~GITIGNORE
+        additions = []
+        additions << "**/generated/**" unless gitignore_content.include?("**/generated/**")
+        additions << "ssr-generated" unless gitignore_content.include?("ssr-generated")
+
+        return if additions.empty?
 
-            # Generated React on Rails packs
-            **/generated/**
-          GITIGNORE
+        append_to_file ".gitignore" do
+          lines = ["\n# Generated React on Rails packs"]
+          lines.concat(additions)
+          lines.join("\n") + "\n"
         end
       end
 

lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt

@@ -43,6 +43,15 @@ ReactOnRails.configure do |config|
   #
   config.server_bundle_js_file = "server-bundle.js"
 
+  # Configure where server bundles are output. Defaults to "ssr-generated".
+  # This should match your webpack configuration for server bundles.
+  config.server_bundle_output_path = "ssr-generated"
+
+  # Enforce that server bundles are only loaded from secure (non-public) directories.
+  # When true, server bundles will only be loaded from the configured server_bundle_output_path.
+  # This is recommended for production to prevent server-side code from being exposed.
+  config.enforce_secure_server_bundles = true
+
   ################################################################################
   ################################################################################
   # FILE SYSTEM BASED COMPONENT REGISTRY

lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt

@@ -44,13 +44,14 @@ const configureServer = () => {
 
   // Custom output for the server-bundle that matches the config in
   // config/initializers/react_on_rails.rb
+  // Server bundles are output to a secure directory (not public) for security
   serverWebpackConfig.output = {
     filename: 'server-bundle.js',
     globalObject: 'this',
     // If using the React on Rails Pro node server renderer, uncomment the next line
     // libraryTarget: 'commonjs2',
-    path: config.outputPath,
-    publicPath: config.publicPath,
+    path: require('path').resolve(__dirname, '../../ssr-generated'),
+    // No publicPath needed since server bundles are not served via web
     // https://webpack.js.org/configuration/output/#outputglobalobject
   };
 

lib/react_on_rails/configuration.rb

@@ -151,6 +151,7 @@ def setup_config_values
       adjust_precompile_task
       check_component_registry_timeout
       validate_generated_component_packs_loading_strategy
+      validate_enforce_secure_server_bundles
     end
 
     private
@@ -199,6 +200,27 @@ def validate_generated_component_packs_loading_strategy
       raise ReactOnRails::Error, "generated_component_packs_loading_strategy must be either :async, :defer, or :sync"
     end
 
+    def validate_enforce_secure_server_bundles
+      return unless enforce_secure_server_bundles
+
+      # Check if server_bundle_output_path is nil
+      if server_bundle_output_path.nil?
+        raise ReactOnRails::Error, "enforce_secure_server_bundles is set to true, but " \
+                                   "server_bundle_output_path is nil. Please set server_bundle_output_path " \
+                                   "to a directory outside of the public directory."
+      end
+
+      # Check if server_bundle_output_path is inside public directory
+      public_path = Rails.root.join("public").to_s
+      server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)
+
+      if server_output_path.start_with?(public_path)
+        raise ReactOnRails::Error, "enforce_secure_server_bundles is set to true, but " \
+                                   "server_bundle_output_path (#{server_bundle_output_path}) is inside " \
+                                   "the public directory. Please set it to a directory outside of public."
+      end
+    end
+
     def check_autobundling_requirements
       raise_missing_components_subdirectory if auto_load_bundle && !components_subdirectory.present?
       return unless components_subdirectory.present?

lib/react_on_rails/utils.rb

@@ -87,17 +87,15 @@ def self.bundle_js_file_path(bundle_name)
       return File.join(generated_assets_full_path, bundle_name) unless ReactOnRails::PackerUtils.using_packer?
 
       is_server_bundle = server_bundle?(bundle_name)
+      config = ReactOnRails.configuration
 
-      if is_server_bundle
-        # NORMAL CASE for server bundles: Try private non-public locations first
-        private_path = try_private_server_locations(bundle_name)
-        return private_path if private_path
-
-        # If enforcement is enabled and no private path found, skip manifest fallback
-        return handle_missing_manifest_entry(bundle_name, true) if enforce_secure_server_bundles?
+      # If server bundle and server_bundle_output_path is configured, try that first
+      if is_server_bundle && config.server_bundle_output_path.present?
+        server_path = try_server_bundle_output_path(bundle_name)
+        return server_path if server_path
       end
 
-      # For client bundles OR server bundle fallback (when enforcement disabled): Try manifest lookup
+      # Try manifest lookup for all bundles
       begin
         ReactOnRails::PackerUtils.bundle_js_uri_from_packer(bundle_name)
       rescue Shakapacker::Manifest::MissingEntryError
@@ -111,66 +109,30 @@ def self.bundle_js_file_path(bundle_name)
       bundle_name == config.rsc_bundle_js_file
     end
 
-    private_class_method def self.enforce_secure_server_bundles?
-      ReactOnRails.configuration.enforce_secure_server_bundles
-    end
-
-    private_class_method def self.try_private_server_locations(bundle_name)
+    private_class_method def self.try_server_bundle_output_path(bundle_name)
       config = ReactOnRails.configuration
       root_path = Rails.root || "."
 
-      # Primary location from configuration (now defaults to "ssr-generated")
-      candidates = [
-        File.join(root_path, config.server_bundle_output_path, bundle_name)
-      ]
-
-      # Add legacy fallback for backwards compatibility
-      candidates << File.join(root_path, "generated", "server-bundles", bundle_name)
-
-      find_first_existing_path(candidates)
+      # Try the configured server_bundle_output_path
+      path = File.join(root_path, config.server_bundle_output_path, bundle_name)
+      expanded_path = File.expand_path(path)
+      File.exist?(expanded_path) ? expanded_path : nil
     end
 
     private_class_method def self.handle_missing_manifest_entry(bundle_name, is_server_bundle)
       config = ReactOnRails.configuration
       root_path = Rails.root || "."
 
-      # When enforcement is on for server bundles, only use private locations
-      if is_server_bundle && enforce_secure_server_bundles?
-        # Only try private locations, no public fallbacks
+      # For server bundles with server_bundle_output_path configured, use that
+      if is_server_bundle && config.server_bundle_output_path.present?
         return File.expand_path(File.join(root_path, config.server_bundle_output_path, bundle_name))
       end
 
-      # When manifest lookup fails, try multiple fallback locations:
-      # 1. Environment-specific path (e.g., public/webpack/test)
-      # 2. Standard Shakapacker location (public/packs)
-      # 3. Generated assets path (for legacy setups)
-      fallback_locations = [
-        File.join(ReactOnRails::PackerUtils.packer_public_output_path, bundle_name),
-        File.join("public", "packs", bundle_name),
-        File.join(generated_assets_full_path, bundle_name)
-      ].uniq
-
-      # Return the first location where the bundle file actually exists
-      existing_path = find_first_existing_path(fallback_locations)
-      return existing_path if existing_path
-
-      # If none exist, return appropriate default based on bundle type
-      if is_server_bundle
-        # For server bundles, use configured private location as final fallback
-        File.expand_path(File.join(root_path, config.server_bundle_output_path, bundle_name))
-      else
-        # For client bundles, use environment-specific path (original behavior)
-        File.expand_path(fallback_locations.first)
-      end
+      # For client bundles and server bundles without special config, use packer's public path
+      # This returns the environment-specific path configured in shakapacker.yml
+      File.expand_path(File.join(ReactOnRails::PackerUtils.packer_public_output_path, bundle_name))
     end
 
-    private_class_method def self.find_first_existing_path(paths)
-      paths.each do |path|
-        expanded_path = File.expand_path(path)
-        return expanded_path if File.exist?(expanded_path)
-      end
-      nil
-    end
 
     def self.server_bundle_js_file_path
       return @server_bundle_path if @server_bundle_path && !Rails.env.development?

spec/react_on_rails/configuration_spec.rb

@@ -425,6 +425,57 @@ module ReactOnRails
         end
       end
     end
+
+    describe "enforce_secure_server_bundles validation" do
+      context "when enforce_secure_server_bundles is true" do
+        it "raises error when server_bundle_output_path is nil" do
+          expect do
+            ReactOnRails.configure do |config|
+              config.server_bundle_output_path = nil
+              config.enforce_secure_server_bundles = true
+            end
+          end.to raise_error(ReactOnRails::Error, /server_bundle_output_path is nil/)
+        end
+
+        it "raises error when server_bundle_output_path is inside public directory" do
+          expect do
+            ReactOnRails.configure do |config|
+              config.server_bundle_output_path = "public/server-bundles"
+              config.enforce_secure_server_bundles = true
+            end
+          end.to raise_error(ReactOnRails::Error, /is inside the public directory/)
+        end
+
+        it "allows server_bundle_output_path outside public directory" do
+          expect do
+            ReactOnRails.configure do |config|
+              config.server_bundle_output_path = "ssr-generated"
+              config.enforce_secure_server_bundles = true
+            end
+          end.not_to raise_error
+        end
+      end
+
+      context "when enforce_secure_server_bundles is false" do
+        it "allows server_bundle_output_path to be nil" do
+          expect do
+            ReactOnRails.configure do |config|
+              config.server_bundle_output_path = nil
+              config.enforce_secure_server_bundles = false
+            end
+          end.not_to raise_error
+        end
+
+        it "allows server_bundle_output_path inside public directory" do
+          expect do
+            ReactOnRails.configure do |config|
+              config.server_bundle_output_path = "public/server-bundles"
+              config.enforce_secure_server_bundles = false
+            end
+          end.not_to raise_error
+        end
+      end
+    end
   end
 end
 

spec/react_on_rails/utils_spec.rb

@@ -76,8 +76,6 @@ def mock_bundle_configs(server_bundle_name: random_bundle_name, rsc_bundle_name:
         .and_return(server_bundle_name)
       allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
         .and_return(rsc_bundle_name)
-      allow(ReactOnRails).to receive_message_chain("configuration.enforce_secure_server_bundles")
-        .and_return(false)
       allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
         .and_return(nil)
     end
@@ -145,27 +143,14 @@ def mock_dev_server_running
               it { is_expected.to eq("#{packer_public_output_path}/manifest.json") }
             end
 
-            context "when file not in manifest and fallback to standard location" do
+            context "when file not in manifest" do
               before do
                 mock_missing_manifest_entry("webpack-bundle.js")
               end
 
-              let(:standard_path) { File.expand_path(File.join("public", "packs", "webpack-bundle.js")) }
               let(:env_specific_path) { File.join(packer_public_output_path, "webpack-bundle.js") }
 
-              it "returns standard path when bundle exists there" do
-                allow(File).to receive(:exist?).and_call_original
-                allow(File).to receive(:exist?).with(File.expand_path(env_specific_path)).and_return(false)
-                allow(File).to receive(:exist?).with(standard_path).and_return(true)
-
-                result = described_class.bundle_js_file_path("webpack-bundle.js")
-                expect(result).to eq(standard_path)
-              end
-
-              it "returns environment-specific path when no bundle exists anywhere" do
-                allow(File).to receive(:exist?).and_call_original
-                allow(File).to receive(:exist?).and_return(false)
-
+              it "returns environment-specific path" do
                 result = described_class.bundle_js_file_path("webpack-bundle.js")
                 expect(result).to eq(File.expand_path(env_specific_path))
               end
@@ -174,43 +159,46 @@ def mock_dev_server_running
             context "with server bundle (SSR/RSC) file not in manifest" do
               let(:server_bundle_name) { "server-bundle.js" }
               let(:ssr_generated_path) { File.expand_path(File.join("ssr-generated", server_bundle_name)) }
-              let(:generated_server_path) do
-                File.expand_path(File.join("generated", "server-bundles", server_bundle_name))
-              end
 
-              before do
-                mock_missing_manifest_entry(server_bundle_name)
-                allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
-                  .and_return(server_bundle_name)
-                allow(ReactOnRails).to receive_message_chain("configuration.enforce_secure_server_bundles")
-                  .and_return(false)
-                allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
-                  .and_return("ssr-generated")
+              context "with server_bundle_output_path configured" do
+                before do
+                  mock_missing_manifest_entry(server_bundle_name)
+                  allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
+                    .and_return(server_bundle_name)
+                  allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
+                    .and_return("ssr-generated")
+                end
+
+                it "tries configured location first for server bundles" do
+                  allow(File).to receive(:exist?).and_call_original
+                  allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
+
+                  result = described_class.bundle_js_file_path(server_bundle_name)
+                  expect(result).to eq(ssr_generated_path)
+                end
+
+                it "falls back to configured path when no bundle exists" do
+                  allow(File).to receive(:exist?).and_call_original
+                  allow(File).to receive(:exist?).and_return(false)
+
+                  result = described_class.bundle_js_file_path(server_bundle_name)
+                  expect(result).to eq(ssr_generated_path)
+                end
               end
 
-              it "tries secure locations first for server bundles" do
-                allow(File).to receive(:exist?).and_call_original
-                allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
-
-                result = described_class.bundle_js_file_path(server_bundle_name)
-                expect(result).to eq(ssr_generated_path)
-              end
-
-              it "tries generated/server-bundles as second secure location" do
-                allow(File).to receive(:exist?).and_call_original
-                allow(File).to receive(:exist?).with(ssr_generated_path).and_return(false)
-                allow(File).to receive(:exist?).with(generated_server_path).and_return(true)
-
-                result = described_class.bundle_js_file_path(server_bundle_name)
-                expect(result).to eq(generated_server_path)
-              end
-
-              it "falls back to ssr-generated location when no bundle exists anywhere" do
-                allow(File).to receive(:exist?).and_call_original
-                allow(File).to receive(:exist?).and_return(false)
-
-                result = described_class.bundle_js_file_path(server_bundle_name)
-                expect(result).to eq(ssr_generated_path)
+              context "without server_bundle_output_path configured" do
+                before do
+                  mock_missing_manifest_entry(server_bundle_name)
+                  allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
+                    .and_return(server_bundle_name)
+                  allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
+                    .and_return(nil)
+                end
+
+                it "uses packer public output path" do
+                  result = described_class.bundle_js_file_path(server_bundle_name)
+                  expect(result).to eq(File.expand_path(File.join(packer_public_output_path, server_bundle_name)))
+                end
               end
             end
 
@@ -222,13 +210,11 @@ def mock_dev_server_running
                 mock_missing_manifest_entry(rsc_bundle_name)
                 allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
                   .and_return(rsc_bundle_name)
-                allow(ReactOnRails).to receive_message_chain("configuration.enforce_secure_server_bundles")
-                  .and_return(false)
                 allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
                   .and_return("ssr-generated")
               end
 
-              it "treats RSC bundles as server bundles and tries secure locations first" do
+              it "treats RSC bundles as server bundles and tries configured location first" do
                 allow(File).to receive(:exist?).and_call_original
                 allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)