-
Notifications
You must be signed in to change notification settings - Fork 26
Demonstrate that ReactOnRails client-side rendering script CSP compatibility #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughServer-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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this 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
⛔ Files ignored due to path filters (1)
Gemfile.lockis 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-srcandstyle-srcis 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
falseto enforce CSP violations before deploying to production.
| policy.script_src :self, :https | ||
| policy.style_src :self, :https |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- Remove
:httpsand 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- 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.
| policy.script_src :self, :https | |
| policy.style_src :self, :https | |
| policy.script_src :self | |
| policy.style_src :self |
| 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 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- Cryptographically random
- Unique per request (session IDs are reused across requests)
- 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.
| 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.
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
Summary by CodeRabbit