Feat/resilience UI i18n model cooldown - [deferred to v4.0.0-rc1]#2145
Feat/resilience UI i18n model cooldown - [deferred to v4.0.0-rc1]#2145rafacpti23 wants to merge 28 commits into
Conversation
…bal routing/memory customizations)
…hen token missing
![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 introduces a major expansion of the platform under the 'Easy IA' branding, adding a customer-facing landing site and portal, a SaaS management dashboard for plans and customers, and Qdrant integration for semantic memory. Feedback highlights critical security risks including insecure JWT secret fallbacks, hardcoded secrets, and missing API prefixes in the middleware matcher that could lead to broken access control. Further improvements are required for internationalization of error messages, cookie security, and replacing synchronous hashing with asynchronous methods to prevent event loop blocking.
| function getJwtSecret(): Uint8Array { | ||
| return new TextEncoder().encode(process.env.JWT_SECRET || ""); | ||
| } |
There was a problem hiding this comment.
Using an empty string as a fallback for JWT_SECRET is a critical security risk. If the environment variable is missing, the application will use a predictable secret, allowing anyone to forge valid management tokens. The application should throw an error if the secret is not configured.
function getJwtSecret(): Uint8Array {
const secret = process.env.JWT_SECRET;
if (!secret) throw new Error("JWT_SECRET is not configured");
return new TextEncoder().encode(secret);
}| "/codex", | ||
| "/models", | ||
| ], | ||
| matcher: ["/", "/adm/:path*", "/dashboard/:path*", "/api/:path*"], |
There was a problem hiding this comment.
The new middleware matcher is missing several critical API prefixes that were present in the previous version, such as /v1, /chat, /responses, /codex, and /models. Since these routes are often used as entry points (and then rewritten to /api/v1/...), excluding them from the matcher will cause requests to these endpoints to bypass middleware logic, including request ID generation, graceful shutdown checks, and body size limits.
| matcher: ["/", "/adm/:path*", "/dashboard/:path*", "/api/:path*"], | |
| matcher: ["/", "/adm/:path*", "/dashboard/:path*", "/api/:path*", "/v1/:path*", "/v1", "/chat/:path*", "/responses/:path*", "/responses", "/codex/:path*", "/codex", "/models"], |
| const store = await cookies(); | ||
| store.set(getPortalCookieName(), token, { | ||
| httpOnly: true, | ||
| sameSite: "lax", |
There was a problem hiding this comment.
The session cookie is configured with secure: false. This allows the cookie to be transmitted over unencrypted HTTP connections, making it vulnerable to interception. It should be set to true in production environments.
| sameSite: "lax", | |
| secure: process.env.NODE_ENV === "production", |
|
|
||
| function getSecret() { | ||
| return new TextEncoder().encode( | ||
| process.env.SITE_PORTAL_SECRET || "easyia-local-portal-secret-change-me" |
There was a problem hiding this comment.
Hardcoding a fallback secret for JWT signing (easyia-local-portal-secret-change-me) is insecure. If the SITE_PORTAL_SECRET environment variable is missing, the application will use this known string, allowing attackers to forge portal session tokens. The application should require a secret from the environment.
| return modelSyncModulePromise; | ||
| } | ||
|
|
||
| export async function proxy(request: any) { |
There was a problem hiding this comment.
The request parameter is typed as any. Using a specific type like NextRequest from next/server would provide better type safety and IDE support.
| export async function proxy(request: any) { | |
| export async function proxy(request: import("next/server").NextRequest) { |
| return "Nao foi possivel validar os beneficios da assinatura desta conta agora. Confirme se a assinatura do provedor esta ativa e tente novamente em alguns instantes."; | ||
| } | ||
|
|
||
| if ( | ||
| normalized.includes("chatgpt account") && | ||
| normalized.includes("model is not supported") && | ||
| normalized.includes("codex") | ||
| ) { | ||
| return "Este modelo nao esta liberado para esta conta do Codex. Escolha outro modelo compativel ou deixe o fallback do OmniRoute seguir para a proxima opcao."; | ||
| } | ||
|
|
||
| if (normalized.includes("requested model is not supported")) { | ||
| return "O modelo solicitado nao esta disponivel neste provedor ou nesta conta. Escolha outro modelo ou use o fallback automatico para continuar."; | ||
| } | ||
|
|
||
| if (normalized.includes("customer api key is disabled")) { | ||
| return "Esta API key esta temporariamente indisponivel. Verifique no painel se o plano esta ativo, se a renovacao foi concluida ou se a chave foi bloqueada pelo administrador."; | ||
| } | ||
|
|
||
| if (normalized.includes("invalid json response from provider")) { | ||
| return "O provedor retornou uma resposta invalida nesta tentativa. Tente novamente em alguns segundos ou deixe o fallback usar outro provedor/modelo."; | ||
| } | ||
|
|
||
| if (normalized.includes("invalid api key")) { | ||
| return "A API key informada nao foi aceita. Revise a chave configurada e tente novamente."; | ||
| } | ||
|
|
||
| if (statusCode === 401) { | ||
| return "Nao foi possivel autenticar esta chamada no provedor configurado. Revise as credenciais e tente novamente."; | ||
| } | ||
|
|
||
| if (statusCode === 402) { | ||
| return "Esta chamada nao pode ser concluida agora porque a conta do provedor precisa de assinatura, creditos ou validacao de beneficios. Verifique o status da conta e tente novamente."; | ||
| } | ||
|
|
||
| if (statusCode === 403) { | ||
| return "Esta chamada foi recusada pelo provedor. Verifique se esta conta tem permissao para usar este recurso ou modelo."; | ||
| } | ||
|
|
||
| if (statusCode === 404) { | ||
| return "O recurso solicitado nao foi encontrado neste provedor. Confira o modelo, rota ou configuracao usada nesta chamada."; | ||
| } | ||
|
|
||
| if (statusCode === 429) { | ||
| return "O limite de uso desta conta foi atingido no momento. Aguarde alguns instantes ou deixe o fallback tentar outra conexao."; | ||
| } | ||
|
|
||
| if (statusCode && statusCode >= 500) { | ||
| return "O provedor retornou uma falha temporaria. Tente novamente em instantes ou deixe o fallback seguir para a proxima opcao."; |
There was a problem hiding this comment.
The normalizeUserFacingErrorMessage function uses hardcoded Portuguese strings for error messages. As the project supports multiple languages and uses an i18n system (next-intl), these messages should be externalized to the locale JSON files to ensure that users see errors in their preferred language.
| paidUntil: data.paidUntil || null, | ||
| extraTokenCredits: data.extraTokenCredits || 0, | ||
| planId: data.planId || null, | ||
| passwordHash: data.password ? bcrypt.hashSync(data.password, 10) : null, |
There was a problem hiding this comment.
Using bcrypt.hashSync in an asynchronous route handler blocks the event loop, which can lead to performance bottlenecks. It is better to use the asynchronous bcrypt.hash method.
| passwordHash: validation.data.password | ||
| ? bcrypt.hashSync(validation.data.password, 10) | ||
| : undefined, |
There was a problem hiding this comment.
Using bcrypt.hashSync in an asynchronous route handler blocks the event loop. It is recommended to use the asynchronous bcrypt.hash method instead.
|
Hey @rafacpti23, thanks for this contribution! 🚀 We really appreciate the work you've put into these features — the SaaS module, Qdrant integration, model cooldowns, and resilience improvements are all excellent additions. However, given the scope of the changes (~28K+ lines, 100 files), this PR is better suited for the next major release cycle. We don't want to rush such a large feature set into v3.8.0 where it could introduce regressions. Plan: This PR will be deferred to v4.0.0-rc1, where we'll have a dedicated integration window for these features. We'll keep this PR open and track it for the v4.0.0 milestone. In the meantime, the model cooldown feature (PR #2146) has been extracted as a focused PR and will be integrated into v3.8.0 separately. Thanks again for the amazing work! 🙌 |
diegosouzapw
changed the title
|
Thank you for this contribution! After review, this PR has been deferred to the v4.0.0-rc1 milestone as it introduces architectural changes that are better suited for the next major release. We'll revisit it there. We appreciate your work and will keep this open. |
|
Thank you @rafacpti23! Deferred to v4.0.0-rc1 alongside #2143 and #2144 for the full resilience UI + i18n integration. Will be merged with full credit when that cycle opens. |
2 participants
Summary
Related Issues
Validation
npm run lintnpm run test:unitnpm run test:coverage>= 60%for statements, lines, functions, and branchesTests Added Or Updated
Coverage Notes
src/,open-sse/,electron/, orbin/, explain which tests cover the change.Reviewer Notes