Skip to content

Conversation

@Judahmeek
Copy link
Contributor

@Judahmeek Judahmeek commented Nov 5, 2025

This PR adds a CSP to test ReactOnRails scripts against.

To run this demo, view Procfile.dev-static and run its commands in separate terminals or run the entire procfile with foreman or overmind.


This change is Reviewable

Summary by CodeRabbit

  • Chores
    • Activated content security policies to restrict resource loading from external sources
    • Modified component rendering behavior for improved configuration

@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

Server-side rendering behavior disabled for a HelloWorld React component, while a content security policy initializer is activated with specific directives, nonce generator updated to use session IDs, and report-only mode enabled.

Changes

Cohort / File(s) Summary
View rendering configuration
app/views/hello_world/index.html.erb
Disabled server-side prerendering by switching prerender from true to false for HelloWorld React component
Content Security Policy setup
config/initializers/content_security_policy.rb
Activated CSP initializer with policy directives (default-src, font-src, img-src, object-src, script-src, style-src); changed nonce generator from random string to session ID; expanded nonce directives to include style-src; enabled CSP report-only mode

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Nonce generator implementation change (SecureRandom.base64(16) → session ID) warrants verification that session ID is an appropriate security identifier
  • CSP directive configuration requires validation against application's asset and resource loading requirements

Poem

🐰 With prerender now set low,
Security policies gently flow,
Session nonces guard each byte,
Styles and scripts shine so bright,
A safer web awaits below!

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Demonstrate that ReactOnRails client-side rendering script CSP compatibility' accurately reflects the main changes in the PR—activating CSP configuration and switching prerender mode—making it directly related to the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch judahmeek/csp

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa7c5cd and 7602855.

⛔ Files ignored due to path filters (1)
  • Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • app/views/hello_world/index.html.erb (1 hunks)
  • config/initializers/content_security_policy.rb (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
config/initializers/content_security_policy.rb (1)
config/application.rb (1)
  • config (23-31)
🔇 Additional comments (3)
app/views/hello_world/index.html.erb (1)

2-2: LGTM! Change aligns with PR objective.

Switching from server-side to client-side rendering is appropriate for testing ReactOnRails CSR scripts with CSP compatibility.

config/initializers/content_security_policy.rb (2)

23-23: LGTM! Appropriate nonce directive configuration.

Applying nonces to both script-src and style-src is the correct approach for protecting against inline script and style injection.


28-28: Appropriate for demo, but remember to enforce in production.

Report-only mode is suitable for testing CSP compatibility (as per the PR objective), but ensure this is set to false to enforce CSP violations before deploying to production.

Comment on lines +12 to +13
policy.script_src :self, :https
policy.style_src :self, :https
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Overly permissive CSP policy weakens security guarantees.

The :https directive in script-src and style-src allows loading scripts and styles from any HTTPS source, which significantly undermines CSP protection. If an attacker can control any HTTPS URL referenced in your application (e.g., through URL parameters, user input, or compromised third-party sources), they can bypass CSP.

Consider one of these approaches:

  1. Remove :https and rely solely on nonces (once the nonce generator is fixed):
-  policy.script_src  :self, :https
-  policy.style_src   :self, :https
+  policy.script_src  :self
+  policy.style_src   :self
  1. Explicitly allowlist trusted CDN domains:
-  policy.script_src  :self, :https
-  policy.style_src   :self, :https
+  policy.script_src  :self, "https://cdn.jsdelivr.net"
+  policy.style_src   :self, "https://cdn.jsdelivr.net"
📝 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
policy.script_src :self, :https
policy.style_src :self, :https
policy.script_src :self
policy.style_src :self
Suggested change
policy.script_src :self, :https
policy.style_src :self, :https
policy.script_src :self, "https://cdn.jsdelivr.net"
policy.style_src :self, "https://cdn.jsdelivr.net"


# If you are using UJS then enable automatic nonce generation
# Rails.application.config.content_security_policy_nonce_generator = -> request { SecureRandom.base64(16) }
Rails.application.config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

CRITICAL: Session ID as CSP nonce is a major security vulnerability.

Using request.session.id.to_s as a CSP nonce completely undermines CSP protection. CSP nonces must be:

  1. Cryptographically random
  2. Unique per request (session IDs are reused across requests)
  3. Unpredictable (session IDs may be enumerable or leaked)

An attacker who obtains the session ID (through XSS, session fixation, network sniffing, etc.) can inject inline scripts/styles that will bypass CSP validation.

Apply this fix to use a cryptographically secure random nonce:

-Rails.application.config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
+Rails.application.config.content_security_policy_nonce_generator = ->(request) { SecureRandom.base64(16) }
📝 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
Rails.application.config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
Rails.application.config.content_security_policy_nonce_generator = ->(request) { SecureRandom.base64(16) }
🤖 Prompt for AI Agents
In config/initializers/content_security_policy.rb around line 20, replace the
use of request.session.id as the CSP nonce with a cryptographically secure,
per-request random value: generate a fresh secure random nonce (e.g. via
SecureRandom.urlsafe_base64 or SecureRandom.base64 with sufficient entropy)
inside the nonce_generator lambda so it is unique and unpredictable for each
request, do not persist it in the session or elsewhere, and ensure the rest of
your CSP setup uses that generated nonce for the script/style nonces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants