Skip to content

Commit 3a4bcc5

Browse files
justin808claude
andcommitted
fix: validate bundle path before hash computation, and harden paths
- Move validate_bundle_exists! inside RendererCacheHelpers.bundle_sources so the guard runs before pool.server_bundle_hash / pool.rsc_bundle_hash are invoked. Hash computation eventually calls Digest::MD5.file / File.mtime on the bundle path, which raises raw Errno::ENOENT if the file is missing — bypassing the friendly ReactOnRailsPro::Error. Drop the now-redundant validate_bundle_exists! calls in both callers. Add a regression spec that un-stubs server_bundle_hash so the real hash-computation path runs on a missing bundle and asserts ReactOnRailsPro::Error (not Errno::ENOENT). - Expand asset paths against Rails.root rather than Dir.pwd in both required_rsc_asset_paths and the per-asset lookups in copy_assets / symlink_assets. This keeps the Set comparison consistent when the rake task runs from a different cwd (e.g. inside a Docker RUN step). - Snapshot ReactOnRailsPro.configuration once at the top of collect_assets so both reads see the same value and the method makes one dispatch. - Remove dead `extend FileUtils` in PrepareNodeRenderBundles (no longer used after the refactor to explicit FileUtils. prefixes). - Add symmetric RENDERER_BUNDLE_PATH deprecation spec in prepare_node_renderer_bundles_spec.rb. All 20 dummy specs pass; rubocop clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 89d1d52 commit 3a4bcc5

4 files changed

Lines changed: 65 additions & 17 deletions

File tree

react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ def self.call
2121
assets = RendererCacheHelpers.collect_assets
2222
rsc_required_paths = RendererCacheHelpers.required_rsc_asset_paths
2323

24-
RendererCacheHelpers.bundle_sources(pool).each do |src_bundle_path, bundle_hash|
25-
RendererCacheHelpers.validate_bundle_exists!(src_bundle_path, "pre-seeding")
24+
RendererCacheHelpers.bundle_sources(pool, "pre-seeding").each do |src_bundle_path, bundle_hash|
2625
seed_bundle(src_bundle_path, bundle_hash, cache_dir)
2726
# The Node Renderer serves manifests from whichever bundle dir it loaded,
2827
# so both server and RSC dirs need the manifests present.
@@ -48,10 +47,12 @@ def self.seed_bundle(src_path, bundle_hash, cache_dir)
4847
# the renderer to fail at runtime with a hard-to-diagnose error. User-configured
4948
# assets_to_copy are optional and only produce a warning. Required assets are
5049
# matched by expanded path rather than basename so a same-named unrelated entry
51-
# in assets_to_copy cannot trigger a false-positive "required" error.
50+
# in assets_to_copy cannot trigger a false-positive "required" error. Expand
51+
# against Rails.root to match how RendererCacheHelpers.required_rsc_asset_paths
52+
# builds its Set.
5253
def self.copy_assets(assets, bundle_dir, rsc_required_paths)
5354
assets.each do |asset_path|
54-
expanded = File.expand_path(asset_path.to_s)
55+
expanded = File.expand_path(asset_path.to_s, Rails.root)
5556
unless File.exist?(expanded)
5657
if rsc_required_paths.include?(expanded)
5758
raise ReactOnRailsPro::Error, "Required RSC asset not found: #{asset_path}. " \

react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ module ReactOnRailsPro
1010
# restores. The staged layout matches the renderer's runtime cache contract:
1111
# <cache>/<bundleHash>/<bundleHash>.js
1212
class PrepareNodeRenderBundles
13-
extend FileUtils
14-
1513
def self.make_relative_symlink(source, destination)
1614
destination_dir = Pathname.new(destination).dirname
1715
FileUtils.mkdir_p(destination_dir)
@@ -39,8 +37,7 @@ def self.call
3937
assets = RendererCacheHelpers.collect_assets
4038
rsc_required_paths = RendererCacheHelpers.required_rsc_asset_paths
4139

42-
RendererCacheHelpers.bundle_sources(pool).each do |src_bundle_path, bundle_hash|
43-
RendererCacheHelpers.validate_bundle_exists!(src_bundle_path, "pre-staging")
40+
RendererCacheHelpers.bundle_sources(pool, "pre-staging").each do |src_bundle_path, bundle_hash|
4441
bundle_dir = File.join(cache_dir, bundle_hash.to_s)
4542
bundle_dest_path = File.join(bundle_dir, "#{bundle_hash}.js")
4643
make_relative_symlink(src_bundle_path, bundle_dest_path)
@@ -50,10 +47,11 @@ def self.call
5047

5148
# Required assets are matched by expanded path rather than basename so a
5249
# same-named unrelated entry in assets_to_copy cannot trigger a false-
53-
# positive "required" error.
50+
# positive "required" error. Expand against Rails.root to match how
51+
# RendererCacheHelpers.required_rsc_asset_paths builds its Set.
5452
def self.symlink_assets(assets, bundle_dir, rsc_required_paths)
5553
assets.each do |asset_path|
56-
expanded = File.expand_path(asset_path.to_s)
54+
expanded = File.expand_path(asset_path.to_s, Rails.root)
5755
unless File.exist?(expanded)
5856
if rsc_required_paths.include?(expanded)
5957
raise ReactOnRailsPro::Error, "Required RSC asset not found: #{asset_path}. " \

react_on_rails_pro/lib/react_on_rails_pro/renderer_cache_helpers.rb

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,28 @@ module RendererCacheHelpers
1010
module_function
1111

1212
def collect_assets
13-
assets = Array(ReactOnRailsPro.configuration.assets_to_copy).dup
13+
config = ReactOnRailsPro.configuration
14+
assets = Array(config.assets_to_copy).dup
1415

15-
if ReactOnRailsPro.configuration.enable_rsc_support
16+
if config.enable_rsc_support
1617
assets << ReactOnRailsPro::Utils.react_client_manifest_file_path
1718
assets << ReactOnRailsPro::Utils.react_server_client_manifest_file_path
1819
end
1920

2021
assets.compact_blank
2122
end
2223

24+
# Must expand against Rails.root so that callers who expand per-asset paths
25+
# against the same base produce Set-comparable strings. Without an explicit
26+
# base, File.expand_path uses Dir.pwd, which differs in Docker RUN steps
27+
# and would make the Set lookup miss.
2328
def required_rsc_asset_paths
2429
return Set.new unless ReactOnRailsPro.configuration.enable_rsc_support
2530

2631
Set.new(
2732
[
28-
File.expand_path(ReactOnRailsPro::Utils.react_client_manifest_file_path.to_s),
29-
File.expand_path(ReactOnRailsPro::Utils.react_server_client_manifest_file_path.to_s)
33+
File.expand_path(ReactOnRailsPro::Utils.react_client_manifest_file_path.to_s, Rails.root),
34+
File.expand_path(ReactOnRailsPro::Utils.react_server_client_manifest_file_path.to_s, Rails.root)
3035
]
3136
)
3237
end
@@ -42,12 +47,21 @@ def validate_bundle_exists!(path, action_description)
4247
# Resolves bundle sources as [path, hash] pairs so callers can iterate
4348
# without needing to re-call pool methods. `pool` must respond to
4449
# `server_bundle_hash` and (when RSC is enabled) `rsc_bundle_hash`.
45-
def bundle_sources(pool)
46-
sources = [[ReactOnRails::Utils.server_bundle_js_file_path, pool.server_bundle_hash]]
50+
#
51+
# Validates each bundle path exists *before* computing its hash, because
52+
# `pool.server_bundle_hash` eventually calls `Digest::MD5.file` / `File.mtime`
53+
# on the bundle path, which raises raw `Errno::ENOENT` if the file is
54+
# missing — bypassing the friendly `ReactOnRailsPro::Error` message.
55+
def bundle_sources(pool, action_description)
56+
server_bundle_path = ReactOnRails::Utils.server_bundle_js_file_path
57+
validate_bundle_exists!(server_bundle_path, action_description)
58+
sources = [[server_bundle_path, pool.server_bundle_hash]]
4759

4860
return sources unless ReactOnRailsPro.configuration.enable_rsc_support
4961

50-
sources << [ReactOnRailsPro::Utils.rsc_bundle_js_file_path, pool.rsc_bundle_hash]
62+
rsc_bundle_path = ReactOnRailsPro::Utils.rsc_bundle_js_file_path
63+
validate_bundle_exists!(rsc_bundle_path, action_description)
64+
sources << [rsc_bundle_path, pool.rsc_bundle_hash]
5165
sources
5266
end
5367
end

react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,23 @@
9696
end
9797
end
9898

99+
# Regression: bundle existence must be validated before pool.server_bundle_hash
100+
# is invoked, since the hash computation calls File.mtime / Digest::MD5.file on
101+
# the bundle path and would otherwise leak a raw Errno::ENOENT.
102+
context "when server bundle doesn't exist and server_bundle_hash is not stubbed" do
103+
before do
104+
FileUtils.rm_f(server_bundle_path)
105+
pool = ReactOnRailsPro::ServerRenderingPool::NodeRenderingPool
106+
allow(pool).to receive(:server_bundle_hash).and_call_original
107+
pool.instance_variable_set(:@server_bundle_hash, nil)
108+
ReactOnRailsPro::Utils.instance_variable_set(:@bundle_hash, nil)
109+
end
110+
111+
it "raises ReactOnRailsPro::Error rather than a raw Errno::ENOENT" do
112+
expect { pre_stage_cache }.to raise_error(ReactOnRailsPro::Error, /Bundle not found/)
113+
end
114+
end
115+
99116
context "with RENDERER_SERVER_BUNDLE_CACHE_PATH env var" do
100117
let(:custom_cache_dir) { Dir.mktmpdir("renderer-cache-test") }
101118

@@ -116,6 +133,24 @@
116133
end
117134
end
118135

136+
context "with deprecated RENDERER_BUNDLE_PATH env var" do
137+
let(:custom_cache_dir) { Dir.mktmpdir("renderer-cache-test") }
138+
139+
before do
140+
ENV["RENDERER_BUNDLE_PATH"] = custom_cache_dir
141+
allow(ReactOnRailsPro.configuration).to receive(:assets_to_copy).and_return(nil)
142+
end
143+
144+
after { FileUtils.rm_rf(custom_cache_dir) }
145+
146+
it "uses the deprecated env var with a warning" do
147+
expect { pre_stage_cache }.to output(/RENDERER_BUNDLE_PATH is deprecated/).to_stderr
148+
149+
dest_file = File.join(custom_cache_dir, bundle_hash, "#{bundle_hash}.js")
150+
expect(File.exist?(dest_file)).to be(true)
151+
end
152+
end
153+
119154
context "when RSC support is enabled" do
120155
let(:rsc_bundle_path) { Rails.root.join("public", "webpack", "production", "rsc-bundle.js").to_s }
121156
let(:rsc_bundle_hash) { "rsc-bundle-hash-xyz789" }

0 commit comments

Comments
 (0)