feat(configGuard): block dev-host/port leak into production config#4235
Conversation
Adds DEV_HOSTS (localhost, 127.0.0.1) and DEV_PORTS (3000-8888 range) sets to assertConfigLoaded so any downstream that misconfigures api.host or api.port will get a hard build-time error instead of a silent prod outage. Guard is a no-op outside production mode. Adds 6 new unit tests covering localhost/127.0.0.1 host rejection, dev-port rejection (3010, 3000), and valid-config pass-through. Incident reference: trawl_vue #949 (signin-broken :3010 outage). Closes #4234. Partially closes pierreb-projects/infra#38 (T11).
|
Warning Review limit reached
More reviews will be available in 48 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe PR strengthens production-build safety by adding validation guards to ChangesProduction config leak detection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 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
🤖 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 `@src/lib/helpers/configGuard.js`:
- Around line 44-45: Normalize api.host and api.port before checking DEV sets:
trim and lower-case the host (e.g., transform apiHost via
String(apiHost).trim().toLowerCase()) and trim the port (or parse/normalize to a
canonical string) before membership checks against DEV_HOSTS and DEV_PORTS.
Update the conditional branches that reference apiHost, apiPort, DEV_HOSTS, and
DEV_PORTS so they use the normalized values when calling has(...), ensuring
inputs like "LOCALHOST" or " 3000 " are correctly detected as dev defaults.
In `@src/lib/helpers/tests/configGuard.unit.tests.js`:
- Around line 46-79: Add regression tests in the configGuard.unit.tests.js suite
calling assertConfigLoaded in 'production' to ensure normalized inputs are
rejected: add one test passing api.host as "LOCALHOST" (uppercase) and another
passing api.port as " 3000 " (padded with whitespace) and assert they throw the
same 'dev-default API host' and 'dev-default API port' errors respectively; use
the existing test patterns (expect(() => assertConfigLoaded(...)).toThrow(...))
so casing and surrounding whitespace are explicitly covered for the host and
port checks.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c11cf125-d763-48e9-89a1-9808962f1e04
📒 Files selected for processing (2)
src/lib/helpers/configGuard.jssrc/lib/helpers/tests/configGuard.unit.tests.js
There was a problem hiding this comment.
Pull request overview
This PR strengthens build-time configuration safety by extending assertConfigLoaded to hard-fail production builds when dev-default API host/port values (e.g., localhost, :3000, :3010) leak into the generated production config.
Changes:
- Add
DEV_HOSTS/DEV_PORTSguards toassertConfigLoaded(production-only) to prevent shipping dev API coordinates. - Extend unit tests to cover localhost / 127.0.0.1 host and common dev ports, plus valid production configurations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/lib/helpers/configGuard.js | Adds DEV host/port leak detection guards to fail production builds with misconfigured API coordinates. |
| src/lib/helpers/tests/configGuard.unit.tests.js | Adds unit tests covering the new production guard behavior for dev hosts/ports and valid configs. |
| if (isPlaceholder) { | ||
| throw new Error( | ||
| 'Production build requires a valid config. Run `npm run config` to generate src/config/index.js before building.', | ||
| ); |
| * Dev-only hostnames and ports that must never reach a production bundle. | ||
| * Root cause of 2026-05-10 signin-broken :3010 outage (issue #949). | ||
| */ | ||
| const DEV_HOSTS = new Set(['localhost', '127.0.0.1']); | ||
| const DEV_PORTS = new Set(['3000', '3001', '3010', '4000', '5000', '8000', '8080', '8888']); |
| throw new Error( | ||
| `Production build has dev-default API host "${apiHost}". ` + | ||
| 'Set DEVKIT_VUE_api_host to the real API URL before building.', | ||
| ); |
…vkit CI Devkit Vue doesn't deploy, so no real API host is set. The new configGuard rejects a localhost default at production build time — add a placeholder value so the guard is satisfied without weakening the downstream check.
configGuard rejects the dev-default port "3000" too. Set it to empty so the production-build guard passes in devkit CI (no real deploy target).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4235 +/- ##
=======================================
Coverage 99.56% 99.57%
=======================================
Files 31 31
Lines 1162 1171 +9
Branches 329 334 +5
=======================================
+ Hits 1157 1166 +9
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Trim + lowercase api.host before DEV_HOSTS.has() — "LOCALHOST" no longer bypasses the guard - Trim api.port string before DEV_PORTS.has() — " 3000 " no longer bypasses - Fix error message: npm run config → npm run generateConfig (matches package.json) - Fix issue ref: #949 → trawl_vue#949 (cross-repo clarity) - Fix error copy: "real API URL" → "host/hostname" (api.host is hostname, not full URL) - Add regression tests for LOCALHOST (uppercase) and " 3000 " (padded) bypass attempts
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
Summary
Promotes the
DEV_HOSTS/DEV_PORTSleak-detection guard from trawl_vue downstream into devkit Vue'ssrc/lib/helpers/configGuard.js. This is generic build-safety infrastructure — any downstream that accidentally configuresapi.host = localhostorapi.port = 3010in a production bundle now gets a hard throw at build time instead of a silent outage.The change is a strict superset of the existing
assertConfigLoadedfunction: all prior behaviour is preserved, two new guard blocks are appended, and the early-return on non-production mode makes the function a clean no-op in dev/test.Incident reference
trawl_vue #949 — signin-broken :3010 outage (2026-05-10): a dev port leaked into the prod bundle because
DEVKIT_VUE_api_portwas not set during the CI build step. Users could not sign in. The guard in this PR would have aborted the build before it shipped.Changes
src/lib/helpers/configGuard.js: addDEV_HOSTS/DEV_PORTSsets + two guard blocks insideassertConfigLoadedsrc/lib/helpers/tests/configGuard.unit.tests.js: add 6 new tests (localhost host, 127.0.0.1 host, port 3010, port 3000, valid host+empty-port, valid host+no-port)Test plan
npm run lint— 0 errorsnpm run test:unit— all 13 configGuard tests pass (38 pre-existing unrelated failures unchanged)Closes
Closes #4234
Partially closes pierreb-projects/infra#38 (Task T11 — devkit side)
Summary by CodeRabbit
Bug Fixes
Tests