Add CORS configuration for Admin SPA#1330
Merged
Merged
Conversation
Enable cross-origin requests from allowed origins to the Admin API v3 endpoints. Origins are dynamically checked against Spree::AllowedOrigin records with a 5-minute cache to avoid per-request DB queries. Scoped to /api/v3/admin/* with an explicit header allowlist. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RackCors as Rack::Cors
participant RailsCache as Rails.cache
participant DB as Spree::AllowedOrigin
Client->>RackCors: Request to /api/v3/admin/...
RackCors->>RackCors: Extract Origin header
RackCors->>RackCors: Validate format & length
RackCors->>RailsCache: fetch("cors/allowed_origin:<origin>")
alt cached
RailsCache-->>RackCors: cached decision (allow/deny)
else not cached
RailsCache->>DB: exists?(origin: <origin>)
DB-->>RailsCache: boolean
RailsCache-->>RackCors: store & return decision
end
alt allowed
RackCors-->>Client: Proceed (CORS headers, credentials allowed)
else denied
RackCors-->>Client: Block or no CORS headers
end
rect rgba(0,128,0,0.5)
Note over RackCors,DB: Errors during DB check are logged to Rails.logger
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
🧹 Nitpick comments (2)
config/initializers/cors.rb (1)
3-7: Nit: multiline lambda style.Rubocop
Style/Lambdaflags line 3. Prefer thelambdakeyword for multiline lambdas (or collapse into theoriginscall). Purely cosmetic.💅 Proposed change
- origins ->(source, _env) { - Rails.cache.fetch("cors/allowed_origin:#{source}", expires_in: 5.minutes) do - Spree::AllowedOrigin.exists?(origin: source) - end - } + origins lambda { |source, _env| + Rails.cache.fetch("cors/allowed_origin:#{source}", expires_in: 5.minutes) do + Spree::AllowedOrigin.exists?(origin: source) + end + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/initializers/cors.rb` around lines 3 - 7, The multiline lambda passed to origins should use the lambda keyword and do/end form instead of the -> literal; update the origins handler (the current ->(source, _env) { Rails.cache.fetch("cors/allowed_origin:#{source}", expires_in: 5.minutes) { Spree::AllowedOrigin.exists?(origin: source) } }) to use lambda do |source, _env| ... end around the Rails.cache.fetch block so RuboCop Style/Lambda is satisfied while preserving the Rails.cache.fetch and Spree::AllowedOrigin.exists? logic.Gemfile (1)
51-51: Optional: pinrack-corsto a known-good major version.Security middleware gems warrant explicit version control; consider using a constraint like
'~> 2.0'to make upgrades intentional and prevent unexpected breaking changes to CORS semantics. The latest release (3.0.0) illustrates why unpinned dependencies can drift across major versions. Not blocking—other gems in this file follow the same pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gemfile` at line 51, The Gemfile currently lists gem 'rack-cors' without a version; pin it to a known-good major version to avoid accidental breaking upgrades by changing the Gemfile entry for 'rack-cors' to include a version constraint (for example '~> 2.0' or another approved major version) so Bundler will keep upgrades intentional and prevent automatic pulls of incompatible v3.x semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/initializers/cors.rb`:
- Around line 1-12: The origins lambda in config/initializers/cors.rb currently
calls Rails.cache.fetch and Spree::AllowedOrigin.exists? which can raise and
cause 5xx responses for all /api/v3/admin/* requests; modify the origins
->(source, _env) { ... } block to rescue storage/DB errors (e.g., StandardError)
around the Rails.cache.fetch / Spree::AllowedOrigin.exists? call, log the error
via Rails.logger.error with context (include source and the exception), and
return false (fail closed) so transient Redis/DB errors deny the origin rather
than crashing the CORS middleware.
- Around line 3-7: The origins lambda currently uses the raw request Origin as
the cache key and DB lookup (Rails.cache.fetch("cors/allowed_origin:#{source}"
...), Spree::AllowedOrigin.exists?(origin: source)), which allows unbounded
cache key growth; before calling Rails.cache.fetch or
Spree::AllowedOrigin.exists? validate and reject attacker-controlled inputs by
returning false for nil/blank values, enforce a reasonable byte-length cap
(e.g., a few hundred bytes), and only accept syntactically valid origins with
http or https schemes (parse and validate the origin); for rejected/invalid
origins skip the cache/DB path and optionally write/lookup a single coarse
negative-cache key (e.g., "cors/allowed_origin:invalid") to collapse repeated
bad requests.
---
Nitpick comments:
In `@config/initializers/cors.rb`:
- Around line 3-7: The multiline lambda passed to origins should use the lambda
keyword and do/end form instead of the -> literal; update the origins handler
(the current ->(source, _env) {
Rails.cache.fetch("cors/allowed_origin:#{source}", expires_in: 5.minutes) {
Spree::AllowedOrigin.exists?(origin: source) } }) to use lambda do |source,
_env| ... end around the Rails.cache.fetch block so RuboCop Style/Lambda is
satisfied while preserving the Rails.cache.fetch and
Spree::AllowedOrigin.exists? logic.
In `@Gemfile`:
- Line 51: The Gemfile currently lists gem 'rack-cors' without a version; pin it
to a known-good major version to avoid accidental breaking upgrades by changing
the Gemfile entry for 'rack-cors' to include a version constraint (for example
'~> 2.0' or another approved major version) so Bundler will keep upgrades
intentional and prevent automatic pulls of incompatible v3.x semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 548d05e0-c52f-4eb5-8a35-fe5e4e27e155
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Gemfileconfig/initializers/cors.rb
- Validate origin format before cache/DB lookup (reject blank, oversized, or non-HTTP(S) origins) to prevent cache pollution - Rescue cache/DB errors and fail closed (deny origin) instead of 500 - Use lambda keyword for multiline lambda (RuboCop Style/Lambda) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/initializers/cors.rb`:
- Around line 3-13: The brace-delimited lambda assigned to origins uses an
inline rescue which is a syntax error; change the lambda from brace form to a
do...end block so the rescue can be used legally. Locate the origins lambda (the
block that checks source.blank?, source.bytesize, the regex match, and the
Rails.cache.fetch("cors/allowed_origin:#{source}", ...) call) and rewrite it as
lambda do |source, _env| ... end, keeping the Rails.cache.fetch block and the
rescue StandardError => e that logs via Rails.logger.error and returns false in
the rescue branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 363542a9-92aa-4473-af2e-3f26401efc36
📒 Files selected for processing (1)
config/initializers/cors.rb
Ruby does not allow rescue inside brace-delimited blocks. Switch
the origins lambda from { } to do...end so the rescue clause parses
correctly.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`origins lambda do...end` causes Ruby to bind the do...end block to `origins` instead of `lambda`, resulting in ArgumentError. Extract the lambda to a local variable to avoid the ambiguous block binding. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
In development, normalize the request Origin to scheme+host (dropping port) before checking against AllowedOrigin records. This lets the Admin SPA on http://localhost:5173 share an AllowedOrigin entry with the storefront on http://localhost:3001 without configuring every dev port. Production keeps strict exact-match behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/initializers/cors.rb`:
- Around line 15-25: The allowlist lookup and cache in the CORS initializer
currently use only origin (variables source/normalized_source) so
Spree::AllowedOrigin.pluck/exists? and Rails.cache.fetch can return an allowed
origin for one store to other stores; update both branches to include the
current store id in the DB query and cache key (e.g. include store_id alongside
origin) so that the Spree::AllowedOrigin lookup (used via
Spree::AllowedOrigin.pluck/exists?) is filtered by store_id and the
Rails.cache.fetch key is suffixed with the store id (use
normalize_origin.call(source)/normalized_source as before but add store id to
the cache key and the ActiveRecord query).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2cccdaa8-4bc3-4da8-8d8f-8c6f9909dc1e
📒 Files selected for processing (1)
config/initializers/cors.rb
The Spree SDK and browsers send headers like Idempotency-Key and DNT that were not in the explicit allowlist, causing CORS preflight failures. Use :any to reflect requested headers — the security boundary is the origin allowlist and server-side API key auth, not the header list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
render
Bot
temporarily deployed
to
feature/rack-cors - Silver Ghost static cloud service 001 PR #1330
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rack-corsmiddleware configuration for Admin SPA cross-origin access to/api/v3/admin/*endpointsSpree::AllowedOriginrecords with a 5-minute cache (Rails.cache)Content-Type,Authorization,Accept,X-Requested-With,X-Spree-Api-KeyTest plan
Spree::AllowedOrigin/api/v3/admin/endpoints cross-origin/api/v3/store/) do not get CORS headers🤖 Generated with Claude Code
Summary by CodeRabbit