Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions lib/react_on_rails/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ def self.configuration
# If exceeded, an error will be thrown for server-side rendered components not registered on the client.
# Set to 0 to disable the timeout and wait indefinitely for component registration.
component_registry_timeout: DEFAULT_COMPONENT_REGISTRY_TIMEOUT,
generated_component_packs_loading_strategy: nil
generated_component_packs_loading_strategy: nil,
# Server bundle security options
server_bundle_output_path: nil,
enforce_private_server_bundles: false
)
end

Expand All @@ -68,7 +71,8 @@ class Configuration
:same_bundle_for_client_and_server, :rendering_props_extension,
:make_generated_server_bundle_the_entrypoint,
:generated_component_packs_loading_strategy, :immediate_hydration, :rsc_bundle_js_file,
:react_client_manifest_file, :react_server_client_manifest_file, :component_registry_timeout
:react_client_manifest_file, :react_server_client_manifest_file, :component_registry_timeout,
:server_bundle_output_path, :enforce_private_server_bundles

# rubocop:disable Metrics/AbcSize
def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender: nil,
Expand All @@ -85,7 +89,7 @@ def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender
random_dom_id: nil, server_render_method: nil, rendering_props_extension: nil,
components_subdirectory: nil, auto_load_bundle: nil, immediate_hydration: nil,
rsc_bundle_js_file: nil, react_client_manifest_file: nil, react_server_client_manifest_file: nil,
component_registry_timeout: nil)
component_registry_timeout: nil, server_bundle_output_path: nil, enforce_private_server_bundles: nil)
self.node_modules_location = node_modules_location.present? ? node_modules_location : Rails.root
self.generated_assets_dirs = generated_assets_dirs
self.generated_assets_dir = generated_assets_dir
Expand Down Expand Up @@ -130,6 +134,8 @@ def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender
self.defer_generated_component_packs = defer_generated_component_packs
self.immediate_hydration = immediate_hydration
self.generated_component_packs_loading_strategy = generated_component_packs_loading_strategy
self.server_bundle_output_path = server_bundle_output_path
self.enforce_private_server_bundles = enforce_private_server_bundles
end
# rubocop:enable Metrics/AbcSize

Expand All @@ -146,6 +152,7 @@ def setup_config_values
adjust_precompile_task
check_component_registry_timeout
validate_generated_component_packs_loading_strategy
validate_enforce_private_server_bundles
end

private
Expand Down Expand Up @@ -194,6 +201,30 @@ 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_private_server_bundles
return unless enforce_private_server_bundles

# Check if server_bundle_output_path is nil
if server_bundle_output_path.nil?
raise ReactOnRails::Error, "enforce_private_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
# Skip validation if Rails.root is not available (e.g., in tests)
return unless defined?(Rails) && Rails.root

public_path = Rails.root.join("public").to_s
server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)

return unless server_output_path.start_with?(public_path)

raise ReactOnRails::Error, "enforce_private_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
Comment on lines +204 to +226
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix public directory containment check (boundary bug).

Using String#start_with? on “/app/public” flags “/app/publication/…” as inside public. Check path boundary.

Apply this diff:

-      public_path = Rails.root.join("public").to_s
-      server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)
-
-      return unless server_output_path.start_with?(public_path)
+      public_path = Rails.root.join("public").expand_path.to_s
+      server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)
+
+      inside_public =
+        server_output_path == public_path ||
+        server_output_path.start_with?("#{public_path}#{File::SEPARATOR}")
+      return unless inside_public

Optional hardening (future): consider using Pathname and existing? checks or realpath with rescue to handle symlinks without requiring path existence.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def validate_enforce_private_server_bundles
return unless enforce_private_server_bundles
# Check if server_bundle_output_path is nil
if server_bundle_output_path.nil?
raise ReactOnRails::Error, "enforce_private_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
# Skip validation if Rails.root is not available (e.g., in tests)
return unless defined?(Rails) && Rails.root
public_path = Rails.root.join("public").to_s
server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)
return unless server_output_path.start_with?(public_path)
raise ReactOnRails::Error, "enforce_private_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
def validate_enforce_private_server_bundles
return unless enforce_private_server_bundles
# Check if server_bundle_output_path is nil
if server_bundle_output_path.nil?
raise ReactOnRails::Error, "enforce_private_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
# Skip validation if Rails.root is not available (e.g., in tests)
return unless defined?(Rails) && Rails.root
public_path = Rails.root.join("public").expand_path.to_s
server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)
inside_public =
server_output_path == public_path ||
server_output_path.start_with?("#{public_path}#{File::SEPARATOR}")
return unless inside_public
raise ReactOnRails::Error, "enforce_private_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
🤖 Prompt for AI Agents
In lib/react_on_rails/configuration.rb around lines 204 to 226, the containment
check uses String#start_with? which treats "/app/publication/..." as inside
"/app/public"; change the check to compare canonical expanded paths so you only
treat paths equal to public or actually nested: expand both
Rails.root.join("public") and server_bundle_output_path, normalize them, then
consider server_output_path inside public only if it equals public_path or
begins with public_path + File::SEPARATOR (i.e., a proper path boundary). Keep
the existing Rails.root guard and nil check; this fixes the boundary bug without
relying on path existence or symlink resolution.


def check_autobundling_requirements
raise_missing_components_subdirectory if auto_load_bundle && !components_subdirectory.present?
return unless components_subdirectory.present?
Expand Down
49 changes: 43 additions & 6 deletions lib/react_on_rails/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,17 @@ def self.server_bundle_path_is_http?
end

def self.bundle_js_file_path(bundle_name)
# Check if this is a server bundle with configured output path - skip manifest lookup
if server_bundle?(bundle_name)
config = ReactOnRails.configuration
root_path = Rails.root || "."

# Use configured server_bundle_output_path if present
if config.server_bundle_output_path.present?
return File.expand_path(File.join(root_path, config.server_bundle_output_path, bundle_name))
end
end

# Either:
# 1. Using same bundle for both server and client, so server bundle will be hashed in manifest
# 2. Using a different bundle (different Webpack config), so file is not hashed, and
Expand All @@ -84,15 +95,12 @@ def self.bundle_js_file_path(bundle_name)
# Default to the non-hashed name in the specified output directory, which, for legacy
# React on Rails, this is the output directory picked up by the asset pipeline.
# For Shakapacker, this is the public output path defined in the (shaka/web)packer.yml file.
File.join(generated_assets_full_path, bundle_name)
File.join(public_bundles_full_path, bundle_name)
else
begin
ReactOnRails::PackerUtils.bundle_js_uri_from_packer(bundle_name)
rescue Shakapacker::Manifest::MissingEntryError
File.expand_path(
File.join(ReactOnRails::PackerUtils.packer_public_output_path,
bundle_name)
)
handle_missing_manifest_entry(bundle_name)
end
end
end
Expand Down Expand Up @@ -165,10 +173,15 @@ def self.using_packer_source_path_is_not_defined_and_custom_node_modules?
ReactOnRails.configuration.node_modules_location.present?
end

def self.generated_assets_full_path
def self.public_bundles_full_path
ReactOnRails::PackerUtils.packer_public_output_path
end

# DEPRECATED: Use public_bundles_full_path for clarity about public vs private bundle paths
def self.generated_assets_full_path
public_bundles_full_path
end

def self.gem_available?(name)
Gem.loaded_specs[name].present?
rescue Gem::LoadError
Expand Down Expand Up @@ -274,5 +287,29 @@ def self.default_troubleshooting_section
• 📖 Discussions: https://github.com/shakacode/react_on_rails/discussions
DEFAULT
end

def self.server_bundle?(bundle_name)
bundle_name == ReactOnRails.configuration.server_bundle_js_file ||
bundle_name == ReactOnRails.configuration.rsc_bundle_js_file
end

def self.enforce_private_server_bundles?
ReactOnRails.configuration.enforce_private_server_bundles
end

def self.handle_missing_manifest_entry(bundle_name)
# For server bundles with enforcement enabled, don't fall back to public paths
if server_bundle?(bundle_name) && enforce_private_server_bundles?
raise Shakapacker::Manifest::MissingEntryError,
"Server bundle '#{bundle_name}' not found in manifest and " \
"enforce_private_server_bundles is enabled. " \
"Ensure server bundle is built and server_bundle_output_path is configured correctly."
end

# Default fallback to non-hashed path in public directory
File.join(public_bundles_full_path, bundle_name)
end

private_class_method :server_bundle?, :enforce_private_server_bundles?, :handle_missing_manifest_entry
end
end
61 changes: 61 additions & 0 deletions spec/react_on_rails/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,67 @@ module ReactOnRails
end
end
end

describe "server bundle security configuration" do
before do
allow(Rails).to receive(:root).and_return(Pathname.new("/app"))
end

describe ".server_bundle_output_path" do
it "has default value of nil" do
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
expect(ReactOnRails.configuration.server_bundle_output_path).to be_nil
end

it "accepts custom path" do
ReactOnRails.configure do |config|
config.server_bundle_output_path = "app/assets/builds"
end
expect(ReactOnRails.configuration.server_bundle_output_path).to eq("app/assets/builds")
end
end

describe ".enforce_private_server_bundles" do
it "has default value of false" do
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
expect(ReactOnRails.configuration.enforce_private_server_bundles).to be(false)
end

it "accepts true value" do
ReactOnRails.configure do |config|
config.server_bundle_output_path = "app/assets/builds"
config.enforce_private_server_bundles = true
end
expect(ReactOnRails.configuration.enforce_private_server_bundles).to be(true)
end

it "raises error when enabled without server_bundle_output_path" do
expect do
ReactOnRails.configure do |config|
config.enforce_private_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/webpack"
config.enforce_private_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 = "app/assets/builds"
config.enforce_private_server_bundles = true
end
end.not_to raise_error
end
end
end
end
end

Expand Down
Loading