Skip to content

Lock down CORS and add rate limiting on the chat endpoints#68

Open
n33levo wants to merge 1 commit into
chat-idor/scope-conversations-to-shopfrom
chat-idor/lock-cors-and-rate-limit
Open

Lock down CORS and add rate limiting on the chat endpoints#68
n33levo wants to merge 1 commit into
chat-idor/scope-conversations-to-shopfrom
chat-idor/lock-cors-and-rate-limit

Conversation

@n33levo

@n33levo n33levo commented Jun 8, 2026

Copy link
Copy Markdown

What this fixes

This follows the IDOR fix in #67 and closes the cross-origin / brute-force pieces called out in Shopify/consumer-agent#1826.

What changed

  • CORS no longer reflects arbitrary origins. Only origins in ALLOWED_ORIGINS are echoed back; localhost browser clients are allowed in development.
  • CORS responses that echo an origin now include Vary: Origin.
  • .env.example now calls out that Shopify storefront origins are required in every environment.
  • Added a small in-memory rate limiter to chat, history, and token-status requests.
  • Rate-limit buckets are scoped per endpoint, use the rightmost forwarded IP instead of client-supplied shop IDs, and do not collapse requests without X-Forwarded-For into a shared unknown bucket.
  • Expired-bucket sweeps are throttled so high-cardinality traffic does not force a full map scan on every request.
  • Chat rate-limit responses and all other pre-stream chat failures now return JSON headers instead of advertising text/event-stream for JSON bodies.
  • The widget checks non-OK chat responses before reading the body as SSE, so HTTP 429 responses surface as a busy message instead of being parsed as stream frames.
  • Token-status now uses the same conversation/shop validation before checking CustomerToken, so it no longer answers questions about raw conversation_id values.
  • The widget sends X-Shopify-Shop-Id when polling token status.
  • Added Node tests for CORS, rate-limit behavior, JSON response contracts, and Vite-backed route behavior.

Stack

  1. Make conversation IDs unguessable and scope chats to their shop #67 — unguessable conversation IDs + shop-scoped conversation access.
  2. This PR — CORS lockdown, rate limiting, and token-status hardening.

Verification

  • node --test — 28 passing, 5 Vite-backed route tests skipped locally because dependencies are not installed in this checkout.
  • Syntax-checked touched JS/JSX/test files with node --check.
  • git diff --check.

@n33levo n33levo requested a review from byrichardpowell June 8, 2026 16:22
@n33levo n33levo force-pushed the chat-idor/scope-conversations-to-shop branch 2 times, most recently from 50cc7e3 to 335e2d8 Compare June 8, 2026 17:46
@n33levo n33levo force-pushed the chat-idor/lock-cors-and-rate-limit branch from 0f84e2f to df80dcf Compare June 8, 2026 17:47

@marcelobarreto marcelobarreto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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


Review assisted by pair-review

Comment thread app/routes/chat.jsx
Comment thread app/routes/chat.jsx
Comment thread app/routes/chat.jsx
Comment thread app/routes/chat.jsx
Comment thread app/services/rate-limiter.server.js
Comment thread app/services/rate-limiter.server.js Outdated
@n33levo n33levo force-pushed the chat-idor/lock-cors-and-rate-limit branch from df80dcf to 103b801 Compare June 8, 2026 23:27
@n33levo n33levo force-pushed the chat-idor/scope-conversations-to-shop branch from 335e2d8 to 030dddd Compare June 8, 2026 23:49
@n33levo n33levo force-pushed the chat-idor/lock-cors-and-rate-limit branch from 103b801 to 92f86f5 Compare June 8, 2026 23:50
@n33levo

n33levo commented Jun 8, 2026

Copy link
Copy Markdown
Author

Follow-up audit after re-reading the stack end-to-end: I pushed 92f86f5 on top of the refreshed #67.

The extra #68 fix is in the widget: it now checks non-OK chat responses before reading the response body as SSE, so HTTP 429 JSON responses surface as the busy message instead of being parsed as stream frames.

Re-verified top of stack: node --test 18/18, git diff --check, syntax checks on touched JS/JSX/test files, plus a grep audit for the risky patterns Marcelo called out.

@n33levo n33levo left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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


Review assisted by pair-review

@n33levo n33levo left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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


Review assisted by pair-review

Comment thread .env.example Outdated
Comment thread app/routes/auth.token-status.jsx
Comment thread app/routes/chat.jsx
Comment thread app/routes/chat.jsx
Comment thread app/services/cors.server.js Outdated
Comment thread app/services/rate-limiter.server.js Outdated
Comment thread app/services/rate-limiter.server.js Outdated
@n33levo n33levo force-pushed the chat-idor/lock-cors-and-rate-limit branch from 92f86f5 to 7db6511 Compare June 9, 2026 20:44
@n33levo n33levo force-pushed the chat-idor/scope-conversations-to-shop branch from b50b09f to 8881405 Compare June 9, 2026 20:45
We were echoing back any Origin (with credentials), so any site could read
chat responses cross-origin. Now we only allow origins from ALLOWED_ORIGINS
(plus localhost in dev). Also added a small in-memory rate limiter on the
chat, history, and token-status routes to slow down brute-force guessing.
@n33levo n33levo force-pushed the chat-idor/lock-cors-and-rate-limit branch from 7db6511 to e88c87e Compare June 9, 2026 20:47
@n33levo n33levo requested a review from siddhantbajaj June 10, 2026 17:15
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