Skip to content

Commit 2a2b663

Browse files
github-actions[bot]AbanoubGhadban
github-actions[bot]
and
AbanoubGhadban
committed
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 <[email protected]>
1 parent 1b750e9 commit 2a2b663

7 files changed

Lines changed: 150 additions & 116 deletions

File tree

‎lib/generators/react_on_rails/base_generator.rb‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -136,14 +136,17 @@ def update_gitignore_for_auto_registration
136136
return unless File.exist?(gitignore_path)
137137

138138
gitignore_content = File.read(gitignore_path)
139-
return if gitignore_content.include?("**/generated/**")
140139

141-
append_to_file ".gitignore" do
142-
<<~GITIGNORE
140+
additions = []
141+
additions << "**/generated/**" unless gitignore_content.include?("**/generated/**")
142+
additions << "ssr-generated" unless gitignore_content.include?("ssr-generated")
143+
144+
return if additions.empty?
143145

144-
# Generated React on Rails packs
145-
**/generated/**
146-
GITIGNORE
146+
append_to_file ".gitignore" do
147+
lines = ["\n# Generated React on Rails packs"]
148+
lines.concat(additions)
149+
lines.join("\n") + "\n"
147150
end
148151
end
149152

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ ReactOnRails.configure do |config|
4343
#
4444
config.server_bundle_js_file = "server-bundle.js"
4545

46+
# Configure where server bundles are output. Defaults to "ssr-generated".
47+
# This should match your webpack configuration for server bundles.
48+
config.server_bundle_output_path = "ssr-generated"
49+
50+
# Enforce that server bundles are only loaded from secure (non-public) directories.
51+
# When true, server bundles will only be loaded from the configured server_bundle_output_path.
52+
# This is recommended for production to prevent server-side code from being exposed.
53+
config.enforce_secure_server_bundles = true
54+
4655
################################################################################
4756
################################################################################
4857
# FILE SYSTEM BASED COMPONENT REGISTRY

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,14 @@ const configureServer = () => {
4444

4545
// Custom output for the server-bundle that matches the config in
4646
// config/initializers/react_on_rails.rb
47+
// Server bundles are output to a secure directory (not public) for security
4748
serverWebpackConfig.output = {
4849
filename: 'server-bundle.js',
4950
globalObject: 'this',
5051
// If using the React on Rails Pro node server renderer, uncomment the next line
5152
// libraryTarget: 'commonjs2',
52-
path: config.outputPath,
53-
publicPath: config.publicPath,
53+
path: require('path').resolve(__dirname, '../../ssr-generated'),
54+
// No publicPath needed since server bundles are not served via web
5455
// https://webpack.js.org/configuration/output/#outputglobalobject
5556
};
5657

‎lib/react_on_rails/configuration.rb‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ def setup_config_values
151151
adjust_precompile_task
152152
check_component_registry_timeout
153153
validate_generated_component_packs_loading_strategy
154+
validate_enforce_secure_server_bundles
154155
end
155156

156157
private
@@ -199,6 +200,27 @@ def validate_generated_component_packs_loading_strategy
199200
raise ReactOnRails::Error, "generated_component_packs_loading_strategy must be either :async, :defer, or :sync"
200201
end
201202

203+
def validate_enforce_secure_server_bundles
204+
return unless enforce_secure_server_bundles
205+
206+
# Check if server_bundle_output_path is nil
207+
if server_bundle_output_path.nil?
208+
raise ReactOnRails::Error, "enforce_secure_server_bundles is set to true, but " \
209+
"server_bundle_output_path is nil. Please set server_bundle_output_path " \
210+
"to a directory outside of the public directory."
211+
end
212+
213+
# Check if server_bundle_output_path is inside public directory
214+
public_path = Rails.root.join("public").to_s
215+
server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)
216+
217+
if server_output_path.start_with?(public_path)
218+
raise ReactOnRails::Error, "enforce_secure_server_bundles is set to true, but " \
219+
"server_bundle_output_path (#{server_bundle_output_path}) is inside " \
220+
"the public directory. Please set it to a directory outside of public."
221+
end
222+
end
223+
202224
def check_autobundling_requirements
203225
raise_missing_components_subdirectory if auto_load_bundle && !components_subdirectory.present?
204226
return unless components_subdirectory.present?

‎lib/react_on_rails/utils.rb‎

Lines changed: 16 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,15 @@ def self.bundle_js_file_path(bundle_name)
8787
return File.join(generated_assets_full_path, bundle_name) unless ReactOnRails::PackerUtils.using_packer?
8888

8989
is_server_bundle = server_bundle?(bundle_name)
90+
config = ReactOnRails.configuration
9091

91-
if is_server_bundle
92-
# NORMAL CASE for server bundles: Try private non-public locations first
93-
private_path = try_private_server_locations(bundle_name)
94-
return private_path if private_path
95-
96-
# If enforcement is enabled and no private path found, skip manifest fallback
97-
return handle_missing_manifest_entry(bundle_name, true) if enforce_secure_server_bundles?
92+
# If server bundle and server_bundle_output_path is configured, try that first
93+
if is_server_bundle && config.server_bundle_output_path.present?
94+
server_path = try_server_bundle_output_path(bundle_name)
95+
return server_path if server_path
9896
end
9997

100-
# For client bundles OR server bundle fallback (when enforcement disabled): Try manifest lookup
98+
# Try manifest lookup for all bundles
10199
begin
102100
ReactOnRails::PackerUtils.bundle_js_uri_from_packer(bundle_name)
103101
rescue Shakapacker::Manifest::MissingEntryError
@@ -111,66 +109,30 @@ def self.bundle_js_file_path(bundle_name)
111109
bundle_name == config.rsc_bundle_js_file
112110
end
113111

114-
private_class_method def self.enforce_secure_server_bundles?
115-
ReactOnRails.configuration.enforce_secure_server_bundles
116-
end
117-
118-
private_class_method def self.try_private_server_locations(bundle_name)
112+
private_class_method def self.try_server_bundle_output_path(bundle_name)
119113
config = ReactOnRails.configuration
120114
root_path = Rails.root || "."
121115

122-
# Primary location from configuration (now defaults to "ssr-generated")
123-
candidates = [
124-
File.join(root_path, config.server_bundle_output_path, bundle_name)
125-
]
126-
127-
# Add legacy fallback for backwards compatibility
128-
candidates << File.join(root_path, "generated", "server-bundles", bundle_name)
129-
130-
find_first_existing_path(candidates)
116+
# Try the configured server_bundle_output_path
117+
path = File.join(root_path, config.server_bundle_output_path, bundle_name)
118+
expanded_path = File.expand_path(path)
119+
File.exist?(expanded_path) ? expanded_path : nil
131120
end
132121

133122
private_class_method def self.handle_missing_manifest_entry(bundle_name, is_server_bundle)
134123
config = ReactOnRails.configuration
135124
root_path = Rails.root || "."
136125

137-
# When enforcement is on for server bundles, only use private locations
138-
if is_server_bundle && enforce_secure_server_bundles?
139-
# Only try private locations, no public fallbacks
126+
# For server bundles with server_bundle_output_path configured, use that
127+
if is_server_bundle && config.server_bundle_output_path.present?
140128
return File.expand_path(File.join(root_path, config.server_bundle_output_path, bundle_name))
141129
end
142130

143-
# When manifest lookup fails, try multiple fallback locations:
144-
# 1. Environment-specific path (e.g., public/webpack/test)
145-
# 2. Standard Shakapacker location (public/packs)
146-
# 3. Generated assets path (for legacy setups)
147-
fallback_locations = [
148-
File.join(ReactOnRails::PackerUtils.packer_public_output_path, bundle_name),
149-
File.join("public", "packs", bundle_name),
150-
File.join(generated_assets_full_path, bundle_name)
151-
].uniq
152-
153-
# Return the first location where the bundle file actually exists
154-
existing_path = find_first_existing_path(fallback_locations)
155-
return existing_path if existing_path
156-
157-
# If none exist, return appropriate default based on bundle type
158-
if is_server_bundle
159-
# For server bundles, use configured private location as final fallback
160-
File.expand_path(File.join(root_path, config.server_bundle_output_path, bundle_name))
161-
else
162-
# For client bundles, use environment-specific path (original behavior)
163-
File.expand_path(fallback_locations.first)
164-
end
131+
# For client bundles and server bundles without special config, use packer's public path
132+
# This returns the environment-specific path configured in shakapacker.yml
133+
File.expand_path(File.join(ReactOnRails::PackerUtils.packer_public_output_path, bundle_name))
165134
end
166135

167-
private_class_method def self.find_first_existing_path(paths)
168-
paths.each do |path|
169-
expanded_path = File.expand_path(path)
170-
return expanded_path if File.exist?(expanded_path)
171-
end
172-
nil
173-
end
174136

175137
def self.server_bundle_js_file_path
176138
return @server_bundle_path if @server_bundle_path && !Rails.env.development?

‎spec/react_on_rails/configuration_spec.rb‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,57 @@ module ReactOnRails
425425
end
426426
end
427427
end
428+
429+
describe "enforce_secure_server_bundles validation" do
430+
context "when enforce_secure_server_bundles is true" do
431+
it "raises error when server_bundle_output_path is nil" do
432+
expect do
433+
ReactOnRails.configure do |config|
434+
config.server_bundle_output_path = nil
435+
config.enforce_secure_server_bundles = true
436+
end
437+
end.to raise_error(ReactOnRails::Error, /server_bundle_output_path is nil/)
438+
end
439+
440+
it "raises error when server_bundle_output_path is inside public directory" do
441+
expect do
442+
ReactOnRails.configure do |config|
443+
config.server_bundle_output_path = "public/server-bundles"
444+
config.enforce_secure_server_bundles = true
445+
end
446+
end.to raise_error(ReactOnRails::Error, /is inside the public directory/)
447+
end
448+
449+
it "allows server_bundle_output_path outside public directory" do
450+
expect do
451+
ReactOnRails.configure do |config|
452+
config.server_bundle_output_path = "ssr-generated"
453+
config.enforce_secure_server_bundles = true
454+
end
455+
end.not_to raise_error
456+
end
457+
end
458+
459+
context "when enforce_secure_server_bundles is false" do
460+
it "allows server_bundle_output_path to be nil" do
461+
expect do
462+
ReactOnRails.configure do |config|
463+
config.server_bundle_output_path = nil
464+
config.enforce_secure_server_bundles = false
465+
end
466+
end.not_to raise_error
467+
end
468+
469+
it "allows server_bundle_output_path inside public directory" do
470+
expect do
471+
ReactOnRails.configure do |config|
472+
config.server_bundle_output_path = "public/server-bundles"
473+
config.enforce_secure_server_bundles = false
474+
end
475+
end.not_to raise_error
476+
end
477+
end
478+
end
428479
end
429480
end
430481

‎spec/react_on_rails/utils_spec.rb‎

Lines changed: 40 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,6 @@ def mock_bundle_configs(server_bundle_name: random_bundle_name, rsc_bundle_name:
7676
.and_return(server_bundle_name)
7777
allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
7878
.and_return(rsc_bundle_name)
79-
allow(ReactOnRails).to receive_message_chain("configuration.enforce_secure_server_bundles")
80-
.and_return(false)
8179
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
8280
.and_return(nil)
8381
end
@@ -145,27 +143,14 @@ def mock_dev_server_running
145143
it { is_expected.to eq("#{packer_public_output_path}/manifest.json") }
146144
end
147145

148-
context "when file not in manifest and fallback to standard location" do
146+
context "when file not in manifest" do
149147
before do
150148
mock_missing_manifest_entry("webpack-bundle.js")
151149
end
152150

153-
let(:standard_path) { File.expand_path(File.join("public", "packs", "webpack-bundle.js")) }
154151
let(:env_specific_path) { File.join(packer_public_output_path, "webpack-bundle.js") }
155152

156-
it "returns standard path when bundle exists there" do
157-
allow(File).to receive(:exist?).and_call_original
158-
allow(File).to receive(:exist?).with(File.expand_path(env_specific_path)).and_return(false)
159-
allow(File).to receive(:exist?).with(standard_path).and_return(true)
160-
161-
result = described_class.bundle_js_file_path("webpack-bundle.js")
162-
expect(result).to eq(standard_path)
163-
end
164-
165-
it "returns environment-specific path when no bundle exists anywhere" do
166-
allow(File).to receive(:exist?).and_call_original
167-
allow(File).to receive(:exist?).and_return(false)
168-
153+
it "returns environment-specific path" do
169154
result = described_class.bundle_js_file_path("webpack-bundle.js")
170155
expect(result).to eq(File.expand_path(env_specific_path))
171156
end
@@ -174,43 +159,46 @@ def mock_dev_server_running
174159
context "with server bundle (SSR/RSC) file not in manifest" do
175160
let(:server_bundle_name) { "server-bundle.js" }
176161
let(:ssr_generated_path) { File.expand_path(File.join("ssr-generated", server_bundle_name)) }
177-
let(:generated_server_path) do
178-
File.expand_path(File.join("generated", "server-bundles", server_bundle_name))
179-
end
180162

181-
before do
182-
mock_missing_manifest_entry(server_bundle_name)
183-
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
184-
.and_return(server_bundle_name)
185-
allow(ReactOnRails).to receive_message_chain("configuration.enforce_secure_server_bundles")
186-
.and_return(false)
187-
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
188-
.and_return("ssr-generated")
163+
context "with server_bundle_output_path configured" do
164+
before do
165+
mock_missing_manifest_entry(server_bundle_name)
166+
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
167+
.and_return(server_bundle_name)
168+
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
169+
.and_return("ssr-generated")
170+
end
171+
172+
it "tries configured location first for server bundles" do
173+
allow(File).to receive(:exist?).and_call_original
174+
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
175+
176+
result = described_class.bundle_js_file_path(server_bundle_name)
177+
expect(result).to eq(ssr_generated_path)
178+
end
179+
180+
it "falls back to configured path when no bundle exists" do
181+
allow(File).to receive(:exist?).and_call_original
182+
allow(File).to receive(:exist?).and_return(false)
183+
184+
result = described_class.bundle_js_file_path(server_bundle_name)
185+
expect(result).to eq(ssr_generated_path)
186+
end
189187
end
190188

191-
it "tries secure locations first for server bundles" do
192-
allow(File).to receive(:exist?).and_call_original
193-
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
194-
195-
result = described_class.bundle_js_file_path(server_bundle_name)
196-
expect(result).to eq(ssr_generated_path)
197-
end
198-
199-
it "tries generated/server-bundles as second secure location" do
200-
allow(File).to receive(:exist?).and_call_original
201-
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(false)
202-
allow(File).to receive(:exist?).with(generated_server_path).and_return(true)
203-
204-
result = described_class.bundle_js_file_path(server_bundle_name)
205-
expect(result).to eq(generated_server_path)
206-
end
207-
208-
it "falls back to ssr-generated location when no bundle exists anywhere" do
209-
allow(File).to receive(:exist?).and_call_original
210-
allow(File).to receive(:exist?).and_return(false)
211-
212-
result = described_class.bundle_js_file_path(server_bundle_name)
213-
expect(result).to eq(ssr_generated_path)
189+
context "without server_bundle_output_path configured" do
190+
before do
191+
mock_missing_manifest_entry(server_bundle_name)
192+
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
193+
.and_return(server_bundle_name)
194+
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
195+
.and_return(nil)
196+
end
197+
198+
it "uses packer public output path" do
199+
result = described_class.bundle_js_file_path(server_bundle_name)
200+
expect(result).to eq(File.expand_path(File.join(packer_public_output_path, server_bundle_name)))
201+
end
214202
end
215203
end
216204

@@ -222,13 +210,11 @@ def mock_dev_server_running
222210
mock_missing_manifest_entry(rsc_bundle_name)
223211
allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
224212
.and_return(rsc_bundle_name)
225-
allow(ReactOnRails).to receive_message_chain("configuration.enforce_secure_server_bundles")
226-
.and_return(false)
227213
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
228214
.and_return("ssr-generated")
229215
end
230216

231-
it "treats RSC bundles as server bundles and tries secure locations first" do
217+
it "treats RSC bundles as server bundles and tries configured location first" do
232218
allow(File).to receive(:exist?).and_call_original
233219
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
234220

0 commit comments

Comments
 (0)