fix: derive the api host, not the console host - #7
Merged
Conversation
`web.<shortcode>.glific.com` mapped to `https://<shortcode>.glific.com/api`, which is the staff
console — a static site that answers any path with index.html and no CORS headers. The browser
reports that as "No 'Access-Control-Allow-Origin' header is present", so it reads as a CORS
misconfiguration when the request is simply reaching the wrong server.
Confirmed on staging:
staging.glific.com/api/v1/web_channel/branding -> 200, index.html
api.staging.glific.com/api/v1/web_channel/branding -> 200, the branding payload
So the widget could not reach any backend once deployed. It now maps to
`https://api.<shortcode>.glific.com/api`. No backend change is needed — that host already answers
with `access-control-allow-origin: *` and a 204 preflight.
The tech design says to map to `<shortcode>.glific.com`; the README and SubdomainPlug's stripping
of an `api.` prefix both say otherwise, and they are right. The original test encoded the design
doc's mapping rather than a reachable host, which is why it passed while the widget was broken —
the added test asserts the base contains `//api.` and is not the console host.
|
@AmishaBisht is attempting to deploy a commit to the Glific Team on Vercel. A member of the Team first needs to authorize it. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 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 |
The first real run of this workflow failed in 19s:
yarn test
vite:plugin:mkcert:WARN The mkcert does not exist, download it now
Error: Request failed [GET] .../mkcert-v1.4.4-linux-amd64 (504 Gateway Time-out)
Vitest resolves the Vite config with `command === "serve"`, same as the dev server, so the
mkcert plugin activated during `yarn test` and tried to download a binary from GitHub. The 504
is incidental — a CI job should not be installing a certificate authority even when the
download succeeds.
Three things resolve this config and none of them wants mkcert: `vite build`, `vite preview`
(Playwright serves the production build over plain HTTP) and `vitest`. The gate covered the
first two; it now covers vitest, with `CI` excluded outright as a backstop.
Verified by running the whole workflow locally with `CI=true` and no `certs/` directory:
typecheck, lint, 48 unit tests, build and 36 e2e all pass with no mkcert activity.
Only reachable now that the workflow actually runs — glific#3 merged it, and this is its first
execution on a PR.
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.
Follow-up to #3. The widget cannot reach any backend once deployed — this fixes that.
The bug
deriveApiBasestrips theweb.prefix and stops there, so it targets the staff console, not the API:Verified against staging:
staging.glific.com/api/v1/web_channel/branding200—<!doctype html>api.staging.glific.com/api/v1/web_channel/branding200—{"data":{"theme":"rose",...}}The console is a static site: it answers any path with
index.htmland a 200, and static hosts send no CORS headers. So the browser reportswhich reads as a CORS misconfiguration when the request is simply going to the wrong server. That misdirection is the main cost of this bug — it sends you looking at
CORSPlugandcheck_origininstead of at the hostname.The fix
No backend change needed.
api.staging.glific.comalready answers withaccess-control-allow-origin: *and a204preflight — I checked both before assuming.Why it shipped
The tech design §4.1 says to map to
<shortcode>.glific.com. The README (https://api.<org>.glific.com/api) andSubdomainPlugstripping anapi.prefix both say otherwise, and they are right. I followed the doc.The test failed to catch it because it asserted the design doc's mapping rather than a reachable host — so it passed while the widget could not talk to staging at all. The added test asserts the base contains
//api.and is explicitly not the console host, which is a property the doc cannot make wrong.Worth correcting §4.1 separately so the next person does not repeat this.
Verification
tsc -b· oxlint · 48 unit tests · 36 e2e across 4 devices — all green.The e2e suite stubs
/brandingwithpage.route, so it never exercises real hostname derivation; that gap is why this needed a unit test rather than a journey.Not in scope
allow-origin: *alongsideallow-credentials: trueis an invalid pairing per the CORS spec. It works today because axios does not send credentials — but if the socket or auth work (#5662 / #5663) setswithCredentials, browsers will reject*and CORS will need configuring properly.