Test CSP with v16 client-side scripts #27
Conversation
WalkthroughThis PR updates project runtime configuration and modifies component rendering behavior. It adds Node.js and Ruby version specifications, disables prerendering for React components, and activates content security policy with nonce-based script and style directives. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.nvmrc(1 hunks).ruby-gemset(1 hunks).ruby-version(1 hunks)app/views/heavy_markdown_editor/index.html.erb(1 hunks)app/views/hello_world/index.html.erb(1 hunks)config/initializers/content_security_policy.rb(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
config/initializers/content_security_policy.rb
[failure] 23-23:
Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
🔇 Additional comments (6)
.ruby-version (1)
1-1: LGTM! Standard Ruby version specification.The Ruby version specification is correct and follows standard conventions.
app/views/heavy_markdown_editor/index.html.erb (1)
1-1: Verify client-side rendering works with CSP nonces.Disabling prerendering aligns with the PR objective of testing CSP with client-side scripts. Ensure that the React component loads correctly with the nonce-based CSP directives configured in the initializer.
.ruby-gemset (1)
1-1: LGTM! Standard gemset specification.The gemset specification follows standard conventions for Ruby version managers.
app/views/hello_world/index.html.erb (1)
1-1: Verify client-side rendering works with CSP nonces.Disabling prerendering is consistent with testing CSP for client-side scripts. Ensure that the React component renders correctly with the configured CSP directives.
config/initializers/content_security_policy.rb (1)
7-17: Good foundation for CSP testing with report-only mode.Enabling report-only mode is a good practice for testing CSP without breaking functionality. The policy structure is well-organized with appropriate directives for different resource types.
Also applies to: 28-28
.nvmrc (1)
1-1: Node.js version 22.12.0 is valid and released.Node.js v22.12.0 is available for download and was released Dec 2–3, 2024. The .nvmrc file is correctly configured.
| policy.script_src :self, :https | ||
| policy.style_src :self, :https |
There was a problem hiding this comment.
Overly permissive CSP directives allow any HTTPS resource.
The :https source in script_src and style_src allows loading scripts and styles from any HTTPS origin, which significantly weakens CSP protection. An attacker who can inject a reference to a malicious HTTPS resource could bypass the policy.
Consider restricting to specific trusted domains or using 'strict-dynamic' with nonces for a more secure approach.
Example of a more restrictive policy:
policy.script_src :self
policy.style_src :selfOr if you need to allow specific CDNs:
policy.script_src :self, 'https://cdn.example.com'
policy.style_src :self, 'https://cdn.example.com'🤖 Prompt for AI Agents
In config/initializers/content_security_policy.rb around lines 12-13, the use of
the generic :https source in policy.script_src and policy.style_src is overly
permissive; replace :https with either only :self or an explicit allowlist of
trusted HTTPS origins (e.g. 'https://cdn.example.com'), or implement
nonce-based/script-hash or 'strict-dynamic' approaches if dynamic third-party
scripts are required; update these two directives to remove the wildcard :https
and include only the minimal trusted sources or nonces as appropriate.
| end | ||
|
|
||
| # If you are using UJS then enable automatic nonce generation | ||
| Rails.application.config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s } |
There was a problem hiding this comment.
Critical: Insecure nonce generation using session ID.
Using request.session.id.to_s as a nonce generator is insecure. Nonces must be cryptographically random and unique per request to prevent CSP bypass attacks. Session IDs are stable within a session and predictable, defeating the purpose of nonces.
Apply this diff to use Rails' built-in secure nonce generator:
-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, the nonce
generator currently uses request.session.id which is insecure; replace it with a
cryptographically secure per-request nonce (e.g., use SecureRandom to generate a
random base64 token or Rails' built-in secure nonce generator) so that nonces
are unique and unpredictable for each request and not derived from the session
ID.
| Rails.application.config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s } | ||
|
|
||
| # Set the nonce only to specific directives | ||
| Rails.application.config.content_security_policy_nonce_directives = %w(script-src style-src) |
There was a problem hiding this comment.
Fix style: Use square brackets for %w literal.
The linter suggests using %w[] instead of %w() for consistency with Ruby style guidelines.
As per coding guidelines.
Apply this diff:
-Rails.application.config.content_security_policy_nonce_directives = %w(script-src style-src)
+Rails.application.config.content_security_policy_nonce_directives = %w[script-src style-src]📝 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_directives = %w(script-src style-src) | |
| Rails.application.config.content_security_policy_nonce_directives = %w[script-src style-src] |
🧰 Tools
🪛 GitHub Check: lint
[failure] 23-23:
Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
🤖 Prompt for AI Agents
In config/initializers/content_security_policy.rb around line 23, the %w() array
literal uses parentheses; replace the parentheses with square brackets to follow
Ruby style guidelines so the line becomes %w[script-src style-src] (keep the
same elements and spacing).
Summary by CodeRabbit
Chores
Configuration
Technical Changes