diff --git a/.agents/workflows/review-prs.md b/.agents/workflows/review-prs.md new file mode 100644 index 0000000000..49feaf35ba --- /dev/null +++ b/.agents/workflows/review-prs.md @@ -0,0 +1,96 @@ +--- +description: Analyze open Pull Requests from the project's GitHub repository, generate a critical report, and optionally implement approved changes +--- + +# /review-prs — PR Review & Analysis Workflow + +## Overview + +This workflow fetches all open PRs from the project's GitHub repository, performs a critical analysis of each one, generates a detailed report, and waits for user approval before proceeding with implementation. + +## Steps + +### 1. Identify the GitHub Repository + +- Read `package.json` to get the repository URL, or use the git remote origin URL + // turbo +- Run: `git -C remote get-url origin` to extract the owner/repo + +### 2. Fetch Open Pull Requests + +- Navigate to `https://github.com///pulls` and scrape all open PRs +- For each open PR, collect: + - PR number, title, author, branch, number of commits, date + - PR description/body + - Files changed (diff) + - Existing review comments (from bots or humans) + +### 3. Analyze Each PR — For each open PR, perform the following analysis: + +#### 3a. Feature Assessment + +- **Does it make sense?** Evaluate if the feature fills a real gap or solves a valid problem +- **Alignment** — Check if it aligns with the project's architecture and roadmap +- **Complexity** — Assess if the scope is reasonable or if it should be split + +#### 3b. Code Quality Review + +- Check for code duplication +- Evaluate error handling patterns (consistent with existing codebase?) +- Check naming conventions and code style +- Verify TypeScript types (any `any` usage, missing types?) + +#### 3c. Security Review + +- Check for missing authentication/authorization on new endpoints +- Check for injection vulnerabilities (URL params, SQL, XSS) +- Verify input validation on all user-controlled data +- Check for hardcoded secrets or credentials + +#### 3d. Architecture Review + +- Does the change follow existing patterns? +- Are there any breaking changes to public APIs? +- Is the database schema affected? Migration needed? +- Impact on performance (N+1 queries, missing indexes?) + +#### 3e. Test Coverage + +- Does the PR include tests? +- Are edge cases covered? +- Would existing tests break? + +### 4. Generate Report — Create a markdown report for each PR including: + +- **PR Summary** — What it does, files affected, commit count +- **Improvements/Benefits** — Numbered list with impact level (HIGH/MEDIUM/LOW) +- **Risks & Issues** — Categorized as CRITICAL / IMPORTANT / MINOR +- **Scoring Table** — Rate across: Feature Relevance, Code Quality, Security, Robustness, Tests +- **Verdict** — Ready to merge? With mandatory vs optional fixes +- **Next Steps** — What will happen if approved + +### 5. Present to User + +- Show the report via `notify_user` with `BlockedOnUser: true` +- Wait for user decision: + - **Approved** → Proceed to step 6 + - **Approved with changes** → Implement the fixes and corrections before merging + - **Rejected** → Close the PR or leave a review comment + +### 6. Implementation (if approved) + +- Checkout the PR branch or apply changes locally +- Implement any required fixes identified in the analysis +- Run the project's test suite to verify nothing breaks + // turbo +- Run: `npm test` or equivalent test command +- Build the project to verify compilation + // turbo +- Run: `npm run build` or equivalent build command +- If all checks pass, prepare the merge + +### 7. Post-Merge (if applicable) + +- Update CHANGELOG.md with the new feature +- Consider version bump if warranted +- Follow the `/generate-release` workflow if a release is needed diff --git a/src/app/(dashboard)/dashboard/combos/page.tsx b/src/app/(dashboard)/dashboard/combos/page.tsx index acf0e8b093..f0ac3c7a5a 100644 --- a/src/app/(dashboard)/dashboard/combos/page.tsx +++ b/src/app/(dashboard)/dashboard/combos/page.tsx @@ -631,8 +631,11 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders }) { const parts = modelValue.split("/"); if (parts.length !== 2) return modelValue; - const [providerId, modelId] = parts; - const matchedNode = providerNodes.find((node) => node.id === providerId); + const [providerIdentifier, modelId] = parts; + // Match by node ID or prefix + const matchedNode = providerNodes.find( + (node) => node.id === providerIdentifier || node.prefix === providerIdentifier + ); if (matchedNode) { return `${matchedNode.name}/${modelId}`; diff --git a/src/app/(dashboard)/dashboard/providers/[id]/page.tsx b/src/app/(dashboard)/dashboard/providers/[id]/page.tsx index ee9116d290..f5f5b2ce74 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/page.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/page.tsx @@ -1,6 +1,7 @@ "use client"; import { useState, useEffect, useCallback, useRef } from "react"; +import { useNotificationStore } from "@/store/notificationStore"; import PropTypes from "prop-types"; import { useParams, useRouter } from "next/navigation"; import Link from "next/link"; @@ -1290,7 +1291,7 @@ function CustomModelsSection({ providerId, providerAlias, copied, onCopy }) { const fetchCustomModels = useCallback(async () => { try { - const res = await fetch(`/api/provider-models?provider=${providerId}`); + const res = await fetch(`/api/provider-models?provider=${encodeURIComponent(providerId)}`); if (res.ok) { const data = await res.json(); setCustomModels(data.models || []); @@ -1334,7 +1335,7 @@ function CustomModelsSection({ providerId, providerAlias, copied, onCopy }) { const handleRemove = async (modelId) => { try { await fetch( - `/api/provider-models?provider=${providerId}&model=${encodeURIComponent(modelId)}`, + `/api/provider-models?provider=${encodeURIComponent(providerId)}&model=${encodeURIComponent(modelId)}`, { method: "DELETE", } @@ -1461,6 +1462,7 @@ function CompatibleModelsSection({ const [newModel, setNewModel] = useState(""); const [adding, setAdding] = useState(false); const [importing, setImporting] = useState(false); + const notify = useNotificationStore(); const providerAliases = Object.entries(modelAliases).filter(([, model]: [string, any]) => (model as string).startsWith(`${providerStorageAlias}/`) @@ -1490,7 +1492,7 @@ function CompatibleModelsSection({ const modelId = newModel.trim(); const resolvedAlias = resolveAlias(modelId); if (!resolvedAlias) { - alert( + notify.error( "All suggested aliases already exist. Please choose a different model or remove conflicting aliases." ); return; @@ -1498,10 +1500,37 @@ function CompatibleModelsSection({ setAdding(true); try { + // Save to customModels DB FIRST - only create alias if this succeeds + const customModelRes = await fetch("/api/provider-models", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + provider: providerStorageAlias, + modelId, + modelName: modelId, + source: "manual", + }), + }); + + if (!customModelRes.ok) { + let errorData: { error?: { message?: string } } = {}; + try { + errorData = await customModelRes.json(); + } catch (jsonError) { + console.error("Failed to parse error response from custom model API:", jsonError); + } + throw new Error(errorData.error?.message || "Failed to save custom model"); + } + + // Only create alias after customModel is saved successfully await onSetAlias(modelId, resolvedAlias, providerStorageAlias); setNewModel(""); + notify.success(`Model ${modelId} added successfully`); } catch (error) { - console.log("Error adding model:", error); + console.error("Error adding model:", error); + notify.error( + error instanceof Error ? error.message : "Failed to add model. Please try again." + ); } finally { setAdding(false); } @@ -1528,12 +1557,32 @@ function CompatibleModelsSection({ if (!modelId) return false; const resolvedAlias = resolveAlias(modelId); if (!resolvedAlias) return false; + + // Save to customModels DB FIRST - only create alias if this succeeds + const customModelRes = await fetch("/api/provider-models", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + provider: providerStorageAlias, + modelId, + modelName: model.name || modelId, + source: "imported", + }), + }); + + if (!customModelRes.ok) { + notify.error("Failed to save imported model to custom database"); + return false; + } + + // Only create alias after customModel is saved successfully await onSetAlias(modelId, resolvedAlias, providerStorageAlias); return true; } ); } catch (error) { - console.log("Error importing models:", error); + console.error("Error importing models:", error); + notify.error("Failed to import models. Please try again."); } finally { setImporting(false); } @@ -1541,6 +1590,28 @@ function CompatibleModelsSection({ const canImport = connections.some((conn) => conn.isActive !== false); + // Handle delete: remove from both alias and customModels DB + const handleDeleteModel = async (modelId: string, alias: string) => { + try { + // Remove from customModels DB + const res = await fetch( + `/api/provider-models?provider=${encodeURIComponent(providerStorageAlias)}&model=${encodeURIComponent(modelId)}`, + { method: "DELETE" } + ); + if (!res.ok) { + throw new Error("Failed to remove model from database"); + } + // Also delete the alias + await onDeleteAlias(alias); + notify.success("Model removed successfully"); + } catch (error) { + console.error("Error deleting model:", error); + notify.error( + error instanceof Error ? error.message : "Failed to delete model. Please try again." + ); + } + }; + return (

@@ -1593,7 +1664,7 @@ function CompatibleModelsSection({ fullModel={`${providerDisplayAlias}/${modelId}`} copied={copied} onCopy={onCopy} - onDeleteAlias={() => onDeleteAlias(alias)} + onDeleteAlias={() => handleDeleteModel(modelId, alias)} /> ))}

diff --git a/src/app/api/models/alias/route.ts b/src/app/api/models/alias/route.ts index 63c8813c0b..002a060dc3 100644 --- a/src/app/api/models/alias/route.ts +++ b/src/app/api/models/alias/route.ts @@ -2,10 +2,16 @@ import { NextResponse } from "next/server"; import { getModelAliases, setModelAlias, deleteModelAlias, isCloudEnabled } from "@/models"; import { getConsistentMachineId } from "@/shared/utils/machineId"; import { syncToCloud } from "@/lib/cloudSync"; +import { isAuthenticated } from "@/shared/utils/apiAuth"; // GET /api/models/alias - Get all aliases -export async function GET() { +export async function GET(request) { try { + // Require authentication for security + if (!(await isAuthenticated(request))) { + return NextResponse.json({ error: "Authentication required" }, { status: 401 }); + } + const aliases = await getModelAliases(); return NextResponse.json({ aliases }); } catch (error) { @@ -17,6 +23,11 @@ export async function GET() { // PUT /api/models/alias - Set model alias export async function PUT(request) { try { + // Require authentication for security + if (!(await isAuthenticated(request))) { + return NextResponse.json({ error: "Authentication required" }, { status: 401 }); + } + const body = await request.json(); const { model, alias } = body; @@ -37,6 +48,11 @@ export async function PUT(request) { // DELETE /api/models/alias?alias=xxx - Delete alias export async function DELETE(request) { try { + // Require authentication for security + if (!(await isAuthenticated(request))) { + return NextResponse.json({ error: "Authentication required" }, { status: 401 }); + } + const { searchParams } = new URL(request.url); const alias = searchParams.get("alias"); diff --git a/src/app/api/provider-models/route.ts b/src/app/api/provider-models/route.ts index d3ce7aa66a..39b1fcf37d 100644 --- a/src/app/api/provider-models/route.ts +++ b/src/app/api/provider-models/route.ts @@ -4,6 +4,7 @@ import { addCustomModel, removeCustomModel, } from "@/lib/localDb"; +import { isAuthenticated } from "@/shared/utils/apiAuth"; /** * GET /api/provider-models?provider= @@ -11,6 +12,14 @@ import { */ export async function GET(request) { try { + // Require authentication for security + if (!(await isAuthenticated(request))) { + return Response.json( + { error: { message: "Authentication required", type: "invalid_api_key" } }, + { status: 401 } + ); + } + const { searchParams } = new URL(request.url); const provider = searchParams.get("provider"); @@ -31,6 +40,14 @@ export async function GET(request) { */ export async function POST(request) { try { + // Require authentication for security + if (!(await isAuthenticated(request))) { + return Response.json( + { error: { message: "Authentication required", type: "invalid_api_key" } }, + { status: 401 } + ); + } + const body = await request.json(); const { provider, modelId, modelName, source } = body; @@ -56,6 +73,14 @@ export async function POST(request) { */ export async function DELETE(request) { try { + // Require authentication for security + if (!(await isAuthenticated(request))) { + return Response.json( + { error: { message: "Authentication required", type: "invalid_api_key" } }, + { status: 401 } + ); + } + const { searchParams } = new URL(request.url); const provider = searchParams.get("provider"); const modelId = searchParams.get("model"); diff --git a/src/app/api/v1/models/route.ts b/src/app/api/v1/models/route.ts index 34364811ac..f938f2618a 100644 --- a/src/app/api/v1/models/route.ts +++ b/src/app/api/v1/models/route.ts @@ -1,10 +1,14 @@ import { CORS_ORIGIN } from "@/shared/utils/cors"; import { PROVIDER_MODELS, PROVIDER_ID_TO_ALIAS } from "@/shared/constants/models"; import { AI_PROVIDERS } from "@/shared/constants/providers"; -import { getProviderConnections, getCombos, getAllCustomModels, getSettings } from "@/lib/localDb"; -import { extractApiKey, isValidApiKey } from "@/sse/services/auth"; -import { jwtVerify } from "jose"; -import { cookies } from "next/headers"; +import { + getProviderConnections, + getCombos, + getAllCustomModels, + getSettings, + getProviderNodes, +} from "@/lib/localDb"; +import { isAuthenticated } from "@/shared/utils/apiAuth"; import { getAllEmbeddingModels } from "@omniroute/open-sse/config/embeddingRegistry.ts"; import { getAllImageModels } from "@omniroute/open-sse/config/imageRegistry.ts"; import { getAllRerankModels } from "@omniroute/open-sse/config/rerankRegistry.ts"; @@ -107,35 +111,7 @@ export async function GET(request: Request) { settings = await getSettings(); } catch {} if (settings.requireAuthForModels === true) { - // Check authentication: API key OR dashboard session (JWT cookie) - // Supports dual auth: Bearer token for external clients, cookie for dashboard. - let isAuthenticated = false; - - // 1. Check API key (for external clients) - const apiKey = extractApiKey(request); - if (apiKey && (await isValidApiKey(apiKey))) { - isAuthenticated = true; - } - - // 2. Check JWT cookie (for dashboard session) - // The auth_token cookie has sameSite:lax + httpOnly, which already - // prevents cross-origin abuse — no additional origin check needed. - // Same pattern as shared/utils/apiAuth.ts verifyAuth(). - if (!isAuthenticated && process.env.JWT_SECRET) { - try { - const cookieStore = await cookies(); - const token = cookieStore.get("auth_token")?.value; - if (token) { - const secret = new TextEncoder().encode(process.env.JWT_SECRET); - await jwtVerify(token, secret); - isAuthenticated = true; - } - } catch { - // Invalid/expired token or cookies not available — not authenticated - } - } - - if (!isAuthenticated) { + if (!(await isAuthenticated(request))) { return Response.json( { error: { @@ -169,6 +145,26 @@ export async function GET(request: Request) { console.log("Could not fetch providers, showing only combos/custom models"); } + // Get provider nodes (for compatible providers with custom prefixes) + let providerNodes = []; + try { + providerNodes = await getProviderNodes(); + } catch (e) { + console.log("Could not fetch provider nodes"); + } + + // Build map of provider node ID to prefix and type for compatible providers + const providerIdToPrefix: Record = {}; + const nodeIdToProviderType: Record = {}; + for (const node of providerNodes) { + if (node.prefix) { + providerIdToPrefix[node.id] = node.prefix; + } + if (node.type) { + nodeIdToProviderType[node.id] = node.type; + } + } + // Get combos let combos = []; try { @@ -318,14 +314,19 @@ export async function GET(request: Request) { try { const customModelsMap: Record = await getAllCustomModels(); for (const [providerId, providerCustomModels] of Object.entries(customModelsMap)) { - const alias = providerIdToAlias[providerId] || providerId; + // For compatible providers, use the prefix from provider nodes + const prefix = providerIdToPrefix[providerId]; + const alias = prefix || providerIdToAlias[providerId] || providerId; const canonicalProviderId = FALLBACK_ALIAS_TO_PROVIDER[alias] || providerId; - // Only include if provider is active — check alias, canonical ID, or raw providerId - // (raw check needed for OpenAI-compatible providers whose ID isn't in the alias map) + + // Only include if provider is active — check alias, canonical ID, raw providerId, + // or the parent provider type (for compatible providers whose node ID is a UUID) + const parentProviderType = nodeIdToProviderType[providerId]; if ( !activeAliases.has(alias) && !activeAliases.has(canonicalProviderId) && - !activeAliases.has(providerId) + !activeAliases.has(providerId) && + !(parentProviderType && activeAliases.has(parentProviderType)) ) continue; @@ -345,7 +346,8 @@ export async function GET(request: Request) { custom: true, }); - if (canonicalProviderId !== alias) { + // Only add provider-prefixed version if different from alias + if (canonicalProviderId !== alias && !prefix) { const providerPrefixedId = `${canonicalProviderId}/${model.id}`; if (models.some((m) => m.id === providerPrefixedId)) continue; models.push({ diff --git a/src/shared/components/ModelSelectModal.tsx b/src/shared/components/ModelSelectModal.tsx index 71b36c33b0..164914a622 100644 --- a/src/shared/components/ModelSelectModal.tsx +++ b/src/shared/components/ModelSelectModal.tsx @@ -149,13 +149,14 @@ export default function ModelSelectModal({ } else if (isCustomProvider) { const matchedNode = providerNodes.find((node) => node.id === providerId); const displayName = matchedNode?.name || providerInfo.name; + const nodePrefix = matchedNode?.prefix || providerId; // Consider a more user-friendly fallback if providerId is a UUID const nodeModels = Object.entries(modelAliases as Record) .filter(([, fullModel]: [string, string]) => fullModel.startsWith(`${providerId}/`)) .map(([aliasName, fullModel]: [string, string]) => ({ id: fullModel.replace(`${providerId}/`, ""), name: aliasName, - value: fullModel, + value: `${nodePrefix}/${fullModel.replace(`${providerId}/`, "")}`, })); // Merge custom models for custom providers @@ -164,7 +165,7 @@ export default function ModelSelectModal({ .map((cm) => ({ id: cm.id, name: cm.name || cm.id, - value: `${providerId}/${cm.id}`, + value: `${nodePrefix}/${cm.id}`, isCustom: true, })); @@ -173,7 +174,7 @@ export default function ModelSelectModal({ if (allModels.length > 0) { groups[providerId] = { name: displayName, - alias: matchedNode?.prefix || providerId, + alias: nodePrefix, color: providerInfo.color, models: allModels, isCustom: true, diff --git a/src/shared/utils/apiAuth.ts b/src/shared/utils/apiAuth.ts index 937315b523..b9e8770192 100644 --- a/src/shared/utils/apiAuth.ts +++ b/src/shared/utils/apiAuth.ts @@ -8,6 +8,7 @@ */ import { jwtVerify } from "jose"; +import { cookies } from "next/headers"; import { getSettings } from "@/lib/localDb"; // ──────────────── Public Routes (No Auth Required) ──────────────── @@ -78,6 +79,46 @@ export async function verifyAuth(request: any): Promise { return "Authentication required"; } +/** + * Check if a request is authenticated — boolean convenience wrapper for route handlers. + * + * Uses `cookies()` from next/headers (App Router compatible) and Bearer API key. + * Returns true if authenticated, false otherwise. + * + * Unlike `verifyAuth`, this does NOT check `isAuthRequired()` — callers that + * need to conditionally skip auth should check that separately. + */ +export async function isAuthenticated(request: Request): Promise { + // 1. Check API key (for external clients) + const authHeader = request.headers.get("authorization"); + if (authHeader?.startsWith("Bearer ")) { + const apiKey = authHeader.slice(7); + try { + const { validateApiKey } = await import("@/lib/db/apiKeys"); + if (await validateApiKey(apiKey)) return true; + } catch { + // DB not ready or import error + } + } + + // 2. Check JWT cookie (for dashboard session) + if (process.env.JWT_SECRET) { + try { + const cookieStore = await cookies(); + const token = cookieStore.get("auth_token")?.value; + if (token) { + const secret = new TextEncoder().encode(process.env.JWT_SECRET); + await jwtVerify(token, secret); + return true; + } + } catch { + // Invalid/expired token or cookies not available + } + } + + return false; +} + /** * Check if a route is in the public (no-auth) allowlist. */ diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 3ff2901338..878c87c9dd 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -156,13 +156,14 @@ export async function handleChat(request: any, clientRawRequest: any = null) { // Pre-check function: skip models where all accounts are in cooldown // Uses modelAvailability module for TTL-based cooldowns const checkModelAvailable = async (modelString: string) => { - const parsed = parseModel(modelString); - const provider = parsed.provider; + // Use getModelInfo to properly resolve custom prefixes + const modelInfo = await getModelInfo(modelString); + const provider = modelInfo.provider; if (!provider) return true; // can't determine provider, let it try // Check domain-level availability (cooldown) - if (!isModelAvailable(provider, parsed.model || modelString)) { - log.debug("AVAILABILITY", `${provider}/${parsed.model} in cooldown, skipping`); + if (!isModelAvailable(provider, modelInfo.model || modelString)) { + log.debug("AVAILABILITY", `${provider}/${modelInfo.model} in cooldown, skipping`); return false; } diff --git a/src/sse/services/model.ts b/src/sse/services/model.ts index effd55ad8a..ec606b7da0 100644 --- a/src/sse/services/model.ts +++ b/src/sse/services/model.ts @@ -22,22 +22,28 @@ export async function resolveModelAlias(alias) { export async function getModelInfo(modelStr) { const parsed = parseModel(modelStr); - if (!parsed.isAlias) { - if (parsed.provider === parsed.providerAlias) { - // Check OpenAI Compatible nodes - const openaiNodes = await getProviderNodes({ type: "openai-compatible" }); - const matchedOpenAI = openaiNodes.find((node) => node.prefix === parsed.providerAlias); - if (matchedOpenAI) { - return { provider: matchedOpenAI.id, model: parsed.model }; - } + // Check custom provider nodes first (for both alias and non-alias formats) + // Check custom provider nodes first (for both alias and non-alias formats) + if (parsed.providerAlias || parsed.provider) { + // Ensure prefixToCheck is always a concise identifier, not a full model string + const prefixToCheck = parsed.providerAlias || parsed.provider; - // Check Anthropic Compatible nodes - const anthropicNodes = await getProviderNodes({ type: "anthropic-compatible" }); - const matchedAnthropic = anthropicNodes.find((node) => node.prefix === parsed.providerAlias); - if (matchedAnthropic) { - return { provider: matchedAnthropic.id, model: parsed.model }; - } + // Check OpenAI Compatible nodes + const openaiNodes = await getProviderNodes({ type: "openai-compatible" }); + const matchedOpenAI = openaiNodes.find((node) => node.prefix === prefixToCheck); + if (matchedOpenAI) { + return { provider: matchedOpenAI.id, model: parsed.model }; } + + // Check Anthropic Compatible nodes + const anthropicNodes = await getProviderNodes({ type: "anthropic-compatible" }); + const matchedAnthropic = anthropicNodes.find((node) => node.prefix === prefixToCheck); + if (matchedAnthropic) { + return { provider: matchedAnthropic.id, model: parsed.model }; + } + } + + if (!parsed.isAlias) { return getModelInfoCore(modelStr, null); }