Skip to content

Commit 0610126

Browse files
justin808claude
andauthored
fix: cross-env validation and docs for renderer password (#3090)
## Summary Addresses all three follow-up items from PR #2829: - **Ruby-side NODE_ENV checking**: `validate_renderer_password_for_production` now checks both `RAILS_ENV` and `NODE_ENV`, mirroring the Node-side `runtimeEnvsAllowDevelopmentDefaults()`. Surfaces misconfigurations (e.g. `NODE_ENV=production` + `RAILS_ENV=development`) at Rails boot time instead of at Node request time. - **Clarifying comment**: Added comment to `defaultReplayServerAsyncOperationLogs()` explaining the intentional asymmetry — it only checks `NODE_ENV` because async log replay is a JS debugging concern, not a security boundary. - **Unconditional validator call**: Moved `validate_renderer_password_for_production` from inside `setup_renderer_password` to `setup_config_values`, making enforcement unconditional and resilient to future refactors of the password resolution logic. Closes #2887 ## Test plan - [x] All 51 Ruby configuration specs pass (including 4 new cross-env tests) - [x] All 31 Node renderer configBuilder tests pass - [x] RuboCop clean - [x] ESLint + Prettier clean - [ ] CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Tightens production-like startup validation for the Node renderer password, which can cause previously misconfigured deployments to fail fast at Rails boot. Changes are localized to configuration/validation paths with expanded spec coverage. > > **Overview** > Ensures `RENDERER_PASSWORD` enforcement is **fail-closed and consistent across Ruby and Node** by validating against both `RAILS_ENV` and `NODE_ENV`, including rejecting mixed-env setups (e.g. `NODE_ENV=production` + `RAILS_ENV=development`). > > Moves `validate_renderer_password_for_production` to run unconditionally during Rails `setup_config_values`, updates the error message docs/matrix accordingly, and expands Ruby specs to cover the new cross-env scenarios. > > Adds a clarifying comment in the Node `configBuilder` that `defaultReplayServerAsyncOperationLogs()` intentionally keys only off `NODE_ENV` (debugging behavior, not a security boundary). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5a01f45. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Strengthened production validation for the Node renderer to prevent misconfiguration in mixed or production envs. * **Documentation** * Clarified default server operation logging behavior and updated environment-related messaging. * **Tests** * Expanded test coverage for environment-variable combinations to ensure consistent behavior across deployment and development scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4d92580 commit 0610126

4 files changed

Lines changed: 100 additions & 27 deletions

File tree

packages/react-on-rails-pro-node-renderer/src/shared/configBuilder.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ function runtimeEnvsAllowDevelopmentDefaults() {
163163
return runtimeEnvs.length > 0 && runtimeEnvs.every((value) => value === 'development' || value === 'test');
164164
}
165165

166+
// Intentionally checks only NODE_ENV, not both NODE_ENV and RAILS_ENV like
167+
// runtimeEnvsAllowDevelopmentDefaults(). Async operation log replay is a JS
168+
// debugging concern, not a security boundary — it should key off the JS
169+
// runtime environment alone.
166170
function defaultReplayServerAsyncOperationLogs() {
167171
if (env.REPLAY_SERVER_ASYNC_OPERATION_LOGS != null) {
168172
return truthy(env.REPLAY_SERVER_ASYNC_OPERATION_LOGS);

react_on_rails_pro/lib/react_on_rails_pro/configuration.rb

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ def setup_config_values
157157
validate_url
158158
validate_remote_bundle_cache_adapter
159159
setup_renderer_password
160+
validate_renderer_password_for_production
160161
setup_assets_to_copy
161162
setup_execjs_profiler_if_needed
162163
check_react_on_rails_support_for_rsc
@@ -260,8 +261,6 @@ def setup_renderer_password
260261
# Mirror Node-side defaults: if Rails config and URL are both missing a password,
261262
# use RENDERER_PASSWORD from env.
262263
self.renderer_password = ENV.fetch("RENDERER_PASSWORD", nil) if renderer_password.blank?
263-
264-
validate_renderer_password_for_production
265264
end
266265

267266
def validate_renderer_password_for_production
@@ -270,12 +269,14 @@ def validate_renderer_password_for_production
270269
return if renderer_password.present?
271270
return unless node_renderer?
272271

273-
# Fail closed: only skip validation when RAILS_ENV is explicitly set to development or test.
274-
# Rails.env defaults to "development" when RAILS_ENV is unset, which would silently skip
275-
# validation in misconfigured environments. Checking ENV["RAILS_ENV"] directly matches the
276-
# Node-side behavior where an unset environment is treated as production-like.
277-
rails_env = ENV["RAILS_ENV"]&.downcase
278-
return if rails_env.present? && %w[development test].include?(rails_env)
272+
# Fail closed: only skip validation when every present runtime env is explicitly
273+
# development or test. This mirrors the Node-side runtimeEnvsAllowDevelopmentDefaults()
274+
# which checks both NODE_ENV and RAILS_ENV. Checking NODE_ENV here surfaces
275+
# misconfigurations (e.g. NODE_ENV=production + RAILS_ENV=development) at Rails boot
276+
# time rather than waiting for the Node renderer to reject the request.
277+
runtime_envs = [ENV.fetch("RAILS_ENV", nil), ENV.fetch("NODE_ENV", nil)].compact_blank.map(&:downcase)
278+
allowed_envs = %w[development test].freeze
279+
return if runtime_envs.any? && runtime_envs.all? { |e| allowed_envs.include?(e) }
279280

280281
raise ReactOnRailsPro::Error, <<~MSG
281282
RENDERER_PASSWORD must be set in production-like environments (staging, production, etc.)
@@ -304,13 +305,12 @@ def validate_renderer_password_for_production
304305
305306
If Rails and the Node Renderer disagree about startup behavior, verify both RAILS_ENV and NODE_ENV.
306307
307-
Environment matrix:
308-
development — password optional (no authentication)
309-
test — password optional (no authentication)
310-
(RAILS_ENV unset) — treated as production-like; RENDERER_PASSWORD required
311-
staging — RENDERER_PASSWORD required
312-
production — RENDERER_PASSWORD required
313-
(any other) — RENDERER_PASSWORD required
308+
Environment matrix (both RAILS_ENV and NODE_ENV are checked):
309+
development/test — password optional when every set env is development or test
310+
(both unset) — treated as production-like; RENDERER_PASSWORD required
311+
staging — RENDERER_PASSWORD required
312+
production — RENDERER_PASSWORD required
313+
(mixed envs) — RENDERER_PASSWORD required (e.g. NODE_ENV=production + RAILS_ENV=development)
314314
MSG
315315
end
316316
end

react_on_rails_pro/sig/react_on_rails_pro/configuration.rbs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,5 +96,7 @@ module ReactOnRailsPro
9696
def validate_remote_bundle_cache_adapter: () -> void
9797

9898
def setup_renderer_password: () -> void
99+
100+
def validate_renderer_password_for_production: () -> void
99101
end
100102
end

react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb

Lines changed: 79 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,9 @@ def self.fetch(*)
163163

164164
it "is blank if not provided in the URL in development" do
165165
allow(ENV).to receive(:[]).and_call_original
166-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("development")
167166
allow(ENV).to receive(:fetch).and_call_original
167+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("development")
168+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return(nil)
168169
allow(ENV).to receive(:fetch).with("RENDERER_PASSWORD", nil).and_return(nil)
169170

170171
ReactOnRailsPro.configure do |config|
@@ -179,10 +180,11 @@ def self.fetch(*)
179180
allow(ENV).to receive(:[]).and_call_original
180181
allow(ENV).to receive(:fetch).and_call_original
181182
allow(ENV).to receive(:fetch).with("RENDERER_PASSWORD", nil).and_return(nil)
183+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return(nil)
182184
end
183185

184186
it "raises an error if no password is set in production" do
185-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("production")
187+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("production")
186188

187189
expect do
188190
ReactOnRailsPro.configure do |config|
@@ -193,7 +195,7 @@ def self.fetch(*)
193195
end
194196

195197
it "raises an error if no password is set in staging" do
196-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("staging")
198+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("staging")
197199

198200
expect do
199201
ReactOnRailsPro.configure do |config|
@@ -204,7 +206,31 @@ def self.fetch(*)
204206
end
205207

206208
it "raises when RAILS_ENV is unset (fail-closed, matching Node-side behavior)" do
207-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return(nil)
209+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return(nil)
210+
211+
expect do
212+
ReactOnRailsPro.configure do |config|
213+
config.server_renderer = "NodeRenderer"
214+
config.renderer_url = "https://localhost:3800"
215+
end
216+
end.to raise_error(ReactOnRailsPro::Error, /RENDERER_PASSWORD must be set/)
217+
end
218+
219+
it "raises when NODE_ENV is production even if RAILS_ENV is development" do
220+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("development")
221+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return("production")
222+
223+
expect do
224+
ReactOnRailsPro.configure do |config|
225+
config.server_renderer = "NodeRenderer"
226+
config.renderer_url = "https://localhost:3800"
227+
end
228+
end.to raise_error(ReactOnRailsPro::Error, /RENDERER_PASSWORD must be set/)
229+
end
230+
231+
it "raises when NODE_ENV is production and RAILS_ENV is unset" do
232+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return(nil)
233+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return("production")
208234

209235
expect do
210236
ReactOnRailsPro.configure do |config|
@@ -215,7 +241,7 @@ def self.fetch(*)
215241
end
216242

217243
it "does not raise when password comes from RENDERER_PASSWORD env var in production" do
218-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("production")
244+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("production")
219245
allow(ENV).to receive(:fetch).with("RENDERER_PASSWORD", nil).and_return("secure-password")
220246

221247
expect do
@@ -229,7 +255,7 @@ def self.fetch(*)
229255
end
230256

231257
it "does not raise when password is explicitly set in production" do
232-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("production")
258+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("production")
233259

234260
expect do
235261
ReactOnRailsPro.configure do |config|
@@ -240,7 +266,7 @@ def self.fetch(*)
240266
end
241267

242268
it "does not raise when password is embedded in the renderer URL in production" do
243-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("production")
269+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("production")
244270

245271
expect do
246272
ReactOnRailsPro.configure do |config|
@@ -251,7 +277,7 @@ def self.fetch(*)
251277
end
252278

253279
it "resolves from ENV when renderer_password is blank in production" do
254-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("production")
280+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("production")
255281
allow(ENV).to receive(:fetch).with("RENDERER_PASSWORD", nil).and_return("secure-password")
256282

257283
expect do
@@ -266,7 +292,7 @@ def self.fetch(*)
266292
end
267293

268294
it "resolves from URL when renderer_password is blank and URL has embedded password" do
269-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("production")
295+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("production")
270296

271297
expect do
272298
ReactOnRailsPro.configure do |config|
@@ -283,10 +309,13 @@ def self.fetch(*)
283309
context "when using NodeRenderer in development/test environments" do
284310
before do
285311
allow(ENV).to receive(:[]).and_call_original
312+
allow(ENV).to receive(:fetch).and_call_original
313+
allow(ENV).to receive(:fetch).with("RENDERER_PASSWORD", nil).and_return(nil)
314+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return(nil)
286315
end
287316

288317
it "does not raise in development even without a password" do
289-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("development")
318+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("development")
290319

291320
expect do
292321
ReactOnRailsPro.configure do |config|
@@ -297,7 +326,43 @@ def self.fetch(*)
297326
end
298327

299328
it "does not raise in test even without a password" do
300-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("test")
329+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("test")
330+
331+
expect do
332+
ReactOnRailsPro.configure do |config|
333+
config.server_renderer = "NodeRenderer"
334+
config.renderer_url = "https://localhost:3800"
335+
end
336+
end.not_to raise_error
337+
end
338+
339+
it "does not raise when RAILS_ENV is unset and NODE_ENV is development" do
340+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return(nil)
341+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return("development")
342+
343+
expect do
344+
ReactOnRailsPro.configure do |config|
345+
config.server_renderer = "NodeRenderer"
346+
config.renderer_url = "https://localhost:3800"
347+
end
348+
end.not_to raise_error
349+
end
350+
351+
it "does not raise when both RAILS_ENV and NODE_ENV are development" do
352+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("development")
353+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return("development")
354+
355+
expect do
356+
ReactOnRailsPro.configure do |config|
357+
config.server_renderer = "NodeRenderer"
358+
config.renderer_url = "https://localhost:3800"
359+
end
360+
end.not_to raise_error
361+
end
362+
363+
it "does not raise when RAILS_ENV is test and NODE_ENV is development" do
364+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("test")
365+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return("development")
301366

302367
expect do
303368
ReactOnRailsPro.configure do |config|
@@ -311,7 +376,9 @@ def self.fetch(*)
311376
context "when using ExecJS renderer" do
312377
it "does not raise in production without a password" do
313378
allow(ENV).to receive(:[]).and_call_original
314-
allow(ENV).to receive(:[]).with("RAILS_ENV").and_return("production")
379+
allow(ENV).to receive(:fetch).and_call_original
380+
allow(ENV).to receive(:fetch).with("RAILS_ENV", nil).and_return("production")
381+
allow(ENV).to receive(:fetch).with("NODE_ENV", nil).and_return(nil)
315382

316383
expect do
317384
ReactOnRailsPro.configure do |config|

0 commit comments

Comments
 (0)