Repository navigation
Refactor split ports - #140
Conversation
- Updated .env.example to include optional split ports for API and dashboard. - Modified docker-compose files to dynamically use the configured ports. - Introduced a new script (run-standalone.mjs) for running the server with environment-specific ports. - Implemented an API bridge server to handle OpenAI-compatible routes when using split ports. - Updated README and CLI tool documentation to reflect changes in port usage and configuration. - Enhanced various components to utilize the new port configuration, ensuring backward compatibility.
- Added API_HOST variable to .env.example for enhanced host configuration. - Updated package.json scripts to utilize a new run-next.mjs script for development and production. - Introduced run-next.mjs to manage Next.js server execution with dynamic port handling. - Enhanced route.ts files to normalize API base URLs and improve configuration checks. - Updated apiBridgeServer.ts to use API_HOST for server initialization. - Expanded global TypeScript definitions to include API_HOST.
Summary of ChangesHello @npmSteven, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the application's port handling mechanism, enabling users to run the API and dashboard on separate network ports. This change provides greater control over network exposure, allowing for API-only deployments and more secure configurations. It involves introducing an API bridge for intelligent request routing and updating various parts of the codebase to support this new, more flexible port architecture. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the application to support splitting the API and dashboard servers onto different ports, introducing wrapper scripts, an API bridge for proxying requests, and a centralized module for port resolution. A critical security vulnerability was identified where the API bridge defaults to listening on all network interfaces (0.0.0.0) and proxies to unauthenticated routes, which could allow unauthorized remote access to the AI proxy. Furthermore, the review highlighted several areas for improvement: duplicated logic for port parsing, a hardcoded port number in an API route, inconsistent command-line argument parsing for ports, and a duplicated Docker healthcheck command. Minor issues like information leakage of port configurations were also noted.
| const { apiPort, dashboardPort } = getRuntimePorts(); | ||
| if (apiPort === dashboardPort) return; | ||
|
|
||
| const host = process.env.API_HOST || "0.0.0.0"; |
There was a problem hiding this comment.
The API bridge server defaults to listening on 0.0.0.0 (all network interfaces), making it accessible to anyone on the same network. This is particularly dangerous because the /api/v1/ routes it proxies to are explicitly excluded from authentication checks in the application's middleware (src/proxy.ts, line 40) and do not implement internal authentication (as evidenced by src/app/api/v1/route.ts). This combination allows unauthorized remote attackers to access the AI proxy, potentially leading to significant financial costs by abusing configured AI providers. It is highly recommended to change the default listening host to 127.0.0.1 to ensure the API bridge is only accessible locally unless explicitly configured otherwise by the user.
| const host = process.env.API_HOST || "0.0.0.0"; | |
| const host = process.env.API_HOST || "127.0.0.1"; |
There was a problem hiding this comment.
@diegosouzapw I've swapped it to 127.0.0.1 for now but if you want it switched to 0.0.0.0 just let me know
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (23 files)
Review Note: This PR introduces a significant architectural change allowing API and dashboard to run on separate ports. The core implementation is solid, and the OAuth config bug has been correctly fixed. The identified warnings should be addressed for production readiness, particularly the missing timeout on proxy requests. |
- Updated .env.example to include optional production ports for API and dashboard. - Modified docker-compose files to utilize dynamic port configuration. - Introduced runtime-env.mjs for centralized port resolution and environment variable management. - Refactored run-next.mjs and run-standalone.mjs to leverage new runtime port handling. - Enhanced route.ts and apiBridgeServer.ts to utilize dynamic ports for improved API integration. - Updated OAuth configuration to reflect changes in port management.
Adds split-port runtime support: API and dashboard can run on separate ports. - PORT remains canonical/base - API_PORT optionally overrides API listener - DASHBOARD_PORT optionally overrides dashboard listener - API bridge server proxies OpenAI-compatible routes with 30s timeout - Centralized port resolution in src/lib/runtime/ports.ts - Unit tests for runtime port resolution (14 tests) - Healthcheck extracted into scripts/healthcheck.mjs
|
✅ Merged to main with additional polish! Improvements applied before merge:
Thank you for this excellent contribution, @npmSteven! 🎉 |
Adds split-port runtime support: API and dashboard can run on separate ports. - PORT remains canonical/base - API_PORT optionally overrides API listener - DASHBOARD_PORT optionally overrides dashboard listener - API bridge server proxies OpenAI-compatible routes with 30s timeout - Centralized port resolution in src/lib/runtime/ports.ts - Unit tests for runtime port resolution (14 tests) - Healthcheck extracted into scripts/healthcheck.mjs
… polish fixes Adds split-port runtime support: API and dashboard can run on separate ports. - PORT remains canonical/base - API_PORT optionally overrides API listener - DASHBOARD_PORT optionally overrides dashboard listener - API bridge server proxies OpenAI-compatible routes with 30s timeout - Centralized port resolution in src/lib/runtime/ports.ts - Unit tests for runtime port resolution (14 tests) - Healthcheck extracted into scripts/healthcheck.mjs
Summary
This PR adds split-port runtime support so OmniRoute can serve the dashboard and OpenAI-compatible API on different ports, while keeping backward compatibility with existing
PORT-based setups.Why
Some deployments need separate listeners for UI and API traffic (network policy, reverse proxies, container routing). Previously, both were tightly coupled to a single port.
What Changed
src/lib/runtime/ports.ts:PORTremains canonical/baseAPI_PORToptionally overrides API listenerDASHBOARD_PORToptionally overrides Next/dashboard listenerOMNIROUTE_PORTpreserves canonical port in wrapped runtimessrc/lib/apiBridgeServer.tsand initialized it insrc/instrumentation.ts:API_PORT !== DASHBOARD_PORT404for non-API routes on API portscripts/run-next.mjs,scripts/run-standalone.mjspackage.jsondev/startnow use wrapper scriptrun-standalone.mjsGET /api/settingsnow returnsruntimePorts,apiPort, anddashboardPortomnirouteManagedmarker.env.exampleand README now document split-port mode and behaviorBackward Compatibility
PORT.API_PORTand/orDASHBOARD_PORT.