Conversation
Signed-off-by: rmeena840 <rmeena840@gmail.com>
Reviewer's GuideAdds server-side geoblocking based on Cloudflare-provided country and region headers, returning a 403 response for sanctioned locations before rendering the app. Sequence diagram for geoblocked request handling on serversequenceDiagram
actor User
participant Browser
participant Cloudflare
participant RemixServer
User->>Browser: Request page
Browser->>Cloudflare: HTTP request
Cloudflare->>Cloudflare: Attach cf_ipcountry and cf_region headers
Cloudflare->>RemixServer: Forward request with headers
RemixServer->>RemixServer: Read cf_ipcountry and cf_region
RemixServer->>RemixServer: Check against SANCTIONED_COUNTRY_REGION
alt Request from sanctioned country or region
RemixServer-->>Cloudflare: 403 Access denied response
Cloudflare-->>Browser: 403 Access denied response
Browser-->>User: Show access denied
else Request from allowed location
RemixServer->>RemixServer: renderToReadableStream
RemixServer-->>Cloudflare: 200 HTML response
Cloudflare-->>Browser: 200 HTML response
Browser-->>User: Render application
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider moving
SANCTIONED_COUNTRY_REGIONinto a shared config or constants module so it can be reused and updated without touching the entry server code. - It might be safer to normalize the
cf-ipcountryandcf-regionheader values (e.g., trim and upper/lowercase) before comparing, to avoid subtle mismatches due to casing or whitespace differences. - The response message 'Access denied: country restricted' could be adjusted to reflect that either the country or region may be restricted, to better match the new logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider moving `SANCTIONED_COUNTRY_REGION` into a shared config or constants module so it can be reused and updated without touching the entry server code.
- It might be safer to normalize the `cf-ipcountry` and `cf-region` header values (e.g., trim and upper/lowercase) before comparing, to avoid subtle mismatches due to casing or whitespace differences.
- The response message 'Access denied: country restricted' could be adjusted to reflect that either the country or region may be restricted, to better match the new logic.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: rmeena840 <rmeena840@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds basic geoblocking at the server entry point to deny access from sanctioned countries and regions based on Cloudflare geolocation headers, addressing compliance requirements from issue #807.
Changes:
- Introduced a
SANCTIONED_COUNTRY_REGIONconfiguration listing sanctioned country ISO codes and specific regions. - Updated
handleRequestinapp/entry.server.tsxto read Cloudflarecf-ipcountryandcf-regionheaders and short‑circuit with a 403 response when a request originates from a sanctioned country or region.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/entry.server.tsx | Adds geolocation header checks and returns a 403 response when the request appears to come from a sanctioned country or region. |
| app/config/sanctions.ts | Defines the static configuration object of sanctioned country codes and regions used by the entrypoint geoblocking logic. |
Signed-off-by: rmeena840 <rmeena840@gmail.com>
| const ipCountryCode = request.headers.get("cf-ipcountry"); | ||
| const ipRegion = request.headers.get("cf-region"); | ||
| const isCountrySanctioned = ipCountryCode && SANCTIONED_COUNTRY_REGION.country.includes(ipCountryCode); | ||
| const isRegionSanctioned = ipRegion && SANCTIONED_COUNTRY_REGION.regions.includes(ipRegion); |
There was a problem hiding this comment.
is there a fallback solution if the header is not included?
There was a problem hiding this comment.
If header is not included than it will be set to null. There is no other fallback solution for this.
Signed-off-by: rmeena840 <rmeena840@gmail.com>
Description
Closes: #807
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues._
I have...
!to the type prefix if API or client breaking changeSummary by Sourcery
New Features: