Uh oh!
There was an error while loading. Please reload this page.
fix(frontend): guard prod builds against staging deploy-config leak - #309
Conversation
The prod `frontend` Cloudflare Workers Build deployed with `--env staging`, baking staging API URLs + a `.staging` cookie domain onto saplinglearn.com, which broke sign-in (middleware routed auth to the staging backend; the session cookie was scoped to `.staging` and never stuck). Add checkFrontendDeployEnv() (src/lib/deployGuard.ts), called from next.config.ts on production builds: fail the build if NEXT_PUBLIC_API_URL / BACKEND_URL / COOKIE_DOMAIN mix environments, or — when DEPLOY_ENV is set — don't match the intended one. Runtime wrangler.toml [vars] can't fix a build that already baked the wrong values, so this catches it at build time. Set DEPLOY_ENV=production on the prod Workers Build and DEPLOY_ENV=staging on the staging one to activate the exact-match check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | dc93218 | Commit Preview URL Branch Preview URL | Jul 03 2026, 02:29 AM |
📝 WalkthroughWalkthroughAdds a new ChangesFrontend deploy environment guard
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/lib/deployGuard.test.ts (1)
33-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for missing/malformed vars under an explicit
DEPLOY_ENVlock.Current cases only exercise wrong-but-canonical values (all-staging on prod) and unknown
DEPLOY_ENV. Consider adding a case whereDEPLOY_ENV=productionbutNEXT_PUBLIC_API_URLis unset or non-canonical (e.g., a typo'd URL) — this currently passes silently per the gap noted indeployGuard.ts, and a test would make that limitation explicit or catch a fix regression.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/lib/deployGuard.test.ts` around lines 33 - 41, Add a test in deployGuard.test.ts that exercises the explicit DEPLOY_ENV lock in checkFrontendDeployEnv with DEPLOY_ENV set to production while NEXT_PUBLIC_API_URL is missing or malformed, so the gap in deployGuard.ts is covered. Use the existing checkFrontendDeployEnv helper and the STAGING/PROD fixtures as the starting point, then assert that the returned problems include the expected missing/invalid URL validation instead of passing silently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/lib/deployGuard.ts`:
- Around line 86-104: The DEPLOY_ENV check in deployGuard.ts only flags entries
already classified by classify(), so missing, blank, or malformed values can
slip through unnoticed. Update the DEPLOY_ENV branch in deployGuard to require
NEXT_PUBLIC_API_URL, BACKEND_URL, and COOKIE_DOMAIN to each classify to the
active deployEnv, and treat any null/omitted classification as a problem. Use
the existing FRONTEND_ENVS, classify, and mismatched/deployEnv logic to report
which of the three keys are absent or not matching the expected environment.
---
Nitpick comments:
In `@frontend/src/lib/deployGuard.test.ts`:
- Around line 33-41: Add a test in deployGuard.test.ts that exercises the
explicit DEPLOY_ENV lock in checkFrontendDeployEnv with DEPLOY_ENV set to
production while NEXT_PUBLIC_API_URL is missing or malformed, so the gap in
deployGuard.ts is covered. Use the existing checkFrontendDeployEnv helper and
the STAGING/PROD fixtures as the starting point, then assert that the returned
problems include the expected missing/invalid URL validation instead of passing
silently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca709c59-9fd9-49be-b01a-e4c99fcc4e10
📒 Files selected for processing (3)
frontend/next.config.tsfrontend/src/lib/deployGuard.test.tsfrontend/src/lib/deployGuard.ts
| const deployEnv = (env.DEPLOY_ENV ?? '').trim().toLowerCase(); | ||
| if (deployEnv) { | ||
| if (!(deployEnv in FRONTEND_ENVS)) { | ||
| problems.push( | ||
| `DEPLOY_ENV must be one of ${Object.keys(FRONTEND_ENVS).join(' | ')}, ` + | ||
| `got ${JSON.stringify(env.DEPLOY_ENV)}`, | ||
| ); | ||
| } else { | ||
| const want = FRONTEND_ENVS[deployEnv as FrontendEnv]; | ||
| const mismatched = Object.entries(classified).filter(([, v]) => v !== deployEnv); | ||
| if (mismatched.length) { | ||
| const detail = mismatched.map(([k, v]) => `${k}→${v}`).join(', '); | ||
| problems.push( | ||
| `DEPLOY_ENV=${deployEnv} but ${detail} point elsewhere; expected ` + | ||
| `NEXT_PUBLIC_API_URL/BACKEND_URL=${want.apiUrl}, COOKIE_DOMAIN=${want.cookieDomain}`, | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
DEPLOY_ENV lock silently passes when a var is missing or malformed, not just wrong-environment.
The mismatch check on Line 95 only inspects classified entries — i.e., values that exactly matched a canonical FRONTEND_ENVS string. If NEXT_PUBLIC_API_URL/BACKEND_URL/COOKIE_DOMAIN is unset, blank, or subtly malformed (trailing slash, wrong subdomain, http vs https), classify() returns null and the key is simply omitted from classified, so it never appears in mismatched and is never flagged. With DEPLOY_ENV=production set, a build where the API URL is accidentally empty or wrong-but-non-canonical will report zero problems — exactly the "silent misconfiguration reaches prod" failure mode this guard exists to prevent, just via a different vector (missing/malformed value) than the original bug (correct-but-wrong-environment value).
Consider requiring all three keys to be classified (matching the intended deployEnv) when DEPLOY_ENV is set, rather than only checking values that already classified successfully:
💡 Proposed fix
} else {
const want = FRONTEND_ENVS[deployEnv as FrontendEnv];
- const mismatched = Object.entries(classified).filter(([, v]) => v !== deployEnv);+ const keys = ['NEXT_PUBLIC_API_URL', 'BACKEND_URL', 'COOKIE_DOMAIN'] as const;+ const mismatched = keys+ .filter((k) => env[k] !== undefined && env[k] !== '')+ .map((k) => [k, classified[k] ?? 'unrecognized'] as const)+ .filter(([, v]) => v !== deployEnv);
if (mismatched.length) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/deployGuard.ts` around lines 86 - 104, The DEPLOY_ENV check
in deployGuard.ts only flags entries already classified by classify(), so
missing, blank, or malformed values can slip through unnoticed. Update the
DEPLOY_ENV branch in deployGuard to require NEXT_PUBLIC_API_URL, BACKEND_URL,
and COOKIE_DOMAIN to each classify to the active deployEnv, and treat any
null/omitted classification as a problem. Use the existing FRONTEND_ENVS,
classify, and mismatched/deployEnv logic to report which of the three keys are
absent or not matching the expected environment.
Uh oh!
There was an error while loading. Please reload this page.
Problem
The prod
frontendCloudflare Workers Build's deploy step wasnpx wrangler deploy --env staging. That appliedwrangler.toml's[env.staging.vars]— staging API URLs + a.staging.saplinglearn.comcookie domain — to the prod worker (wrangler even logged that it overrode the worker namefrontend-staging→frontend). Sign-in onsaplinglearn.combroke: the middleware routed auth to the staging backend and thesapling_sessioncookie was scoped to.staging, so it never stuck.These values bake at build time (
NEXT_PUBLIC_API_URLis inlined; the/apirewrite usesBACKEND_URL), so runtimewrangler.toml [vars]couldn't correct a build that already shipped the wrong values.Fix
Add
checkFrontendDeployEnv()(src/lib/deployGuard.ts), called fromnext.config.tson production builds (alongside the existingBACKEND_URLguard):NEXT_PUBLIC_API_URL/BACKEND_URL/COOKIE_DOMAINdescribe more than one environment (e.g. prod API URL +.stagingcookie domain).DEPLOY_ENV): when set, fail if any value doesn't match that environment — this catches an all-staging build shipped to the prod worker (the exact bug), which the consistency check alone can't detect.Operator follow-up (not code — do in the Cloudflare dashboard)
frontendWorkers Build → Deploy command:npx wrangler deploy --env staging→npx wrangler deploy.DEPLOY_ENV=production(prod build var) andDEPLOY_ENV=staging(staging build var) to activate the exact-match check.Tests
src/lib/deployGuard.test.ts— 9 cases: clean prod/staging, matchingDEPLOY_ENV, split-brain mix, all-staging-on-prod, badDEPLOY_ENV, unset/no-op (local dev), non-canonical preview URLs, whitespace/case tolerance.tsc --noEmit+eslintclean.🤖 Generated with Claude Code
Summary by CodeRabbit