mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 12:22:14 +03:00
fix: security hardening and UX improvements for PR #122
- Fix URL parameter injection: apply encodeURIComponent on providerStorageAlias and providerId in all API calls - Replace blocking alert() with non-blocking notify.error/notify.success toast notifications - Add success feedback for model add and delete operations - Improve error handling: use console.error consistently and add user-facing notifications for import failures - Check DELETE response status before proceeding with alias removal
This commit is contained in:
96
.agents/workflows/review-prs.md
Normal file
96
.agents/workflows/review-prs.md
Normal file
@@ -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 <project_root> remote get-url origin` to extract the owner/repo
|
||||
|
||||
### 2. Fetch Open Pull Requests
|
||||
|
||||
- Navigate to `https://github.com/<owner>/<repo>/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
|
||||
@@ -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;
|
||||
@@ -1523,9 +1525,12 @@ function CompatibleModelsSection({
|
||||
// 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);
|
||||
alert(error instanceof Error ? error.message : "Failed to add model. Please try again.");
|
||||
console.error("Error adding model:", error);
|
||||
notify.error(
|
||||
error instanceof Error ? error.message : "Failed to add model. Please try again."
|
||||
);
|
||||
} finally {
|
||||
setAdding(false);
|
||||
}
|
||||
@@ -1566,7 +1571,7 @@ function CompatibleModelsSection({
|
||||
});
|
||||
|
||||
if (!customModelRes.ok) {
|
||||
console.error("Failed to save imported model to customModels DB");
|
||||
notify.error("Failed to save imported model to custom database");
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -1576,7 +1581,8 @@ function CompatibleModelsSection({
|
||||
}
|
||||
);
|
||||
} 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);
|
||||
}
|
||||
@@ -1588,14 +1594,21 @@ function CompatibleModelsSection({
|
||||
const handleDeleteModel = async (modelId: string, alias: string) => {
|
||||
try {
|
||||
// Remove from customModels DB
|
||||
await fetch(
|
||||
`/api/provider-models?provider=${providerStorageAlias}&model=${encodeURIComponent(modelId)}`,
|
||||
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.log("Error deleting model:", error);
|
||||
console.error("Error deleting model:", error);
|
||||
notify.error(
|
||||
error instanceof Error ? error.message : "Failed to delete model. Please try again."
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user