Skip to content

feat: migrate chat delete flow to dedicated api#1612

Open
arpitgupta1214 wants to merge 1 commit intotestfrom
codex/arpit-migrate-chat-delete-endpoint
Open

feat: migrate chat delete flow to dedicated api#1612
arpitgupta1214 wants to merge 1 commit intotestfrom
codex/arpit-migrate-chat-delete-endpoint

Conversation

@arpitgupta1214
Copy link
Copy Markdown
Collaborator

@arpitgupta1214 arpitgupta1214 commented Mar 30, 2026

Summary

  • migrate sidebar chat delete flow from local POST /api/room/delete to dedicated DELETE /api/chats
  • send bearer auth from frontend using useAccessToken
  • support API override in delete flow using useApiOverride
  • remove local endpoint app/api/room/delete/route.ts

Validation

  • pnpm exec eslint components/Sidebar/Modals/DeleteConfirmationModal.tsx

Summary by CodeRabbit

  • Refactor
    • Updated room deletion functionality to use external API endpoint with explicit authentication requirements. Removed local API handler in favor of authenticated HTTP requests with bearer token validation. Enhanced error handling for deletion operations.

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
recoup-chat Ready Ready Preview Mar 30, 2026 6:31pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

📝 Walkthrough

Walkthrough

The changes migrate room deletion from a local Next.js API route to an external API endpoint. The deprecated local handler is removed, and the client component is updated to call the new authenticated external service with proper bearer token authorization.

Changes

Cohort / File(s) Summary
Local API Removal
app/api/room/delete/route.ts
Entire POST handler and route configuration removed, shifting deletion responsibility to external API.
Client API Migration
components/Sidebar/Modals/DeleteConfirmationModal.tsx
Updated to call external ${baseUrl}/api/chats endpoint instead of local /api/room/delete. Integrated useAccessToken and useApiOverride hooks for authentication. Changed HTTP method from POST to DELETE. Added bearer token authorization header. Enhanced error message handling with priority fallback (result.messageresult.error → default).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • sweetmantech

Poem

🚀 From local routes to distant shores,
Bearer tokens guard the doors.
Delete now speaks in external tongue,
Authentication checks are sung.
One endpoint falls, another's born,
Architecture reborn, refined, reborn. ✨

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning Pull request violates DRY principle through duplicated helper functions and SRP by centralizing API calls, auth, and state management in one component without delegating to utilities. Extract chat helpers to shared utility, create deleteChat function in lib/chats following updateChat pattern, validate API override origins before sending auth tokens, and delegate API calls from component to utilities.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/arpit-migrate-chat-delete-endpoint

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/Sidebar/Modals/DeleteConfirmationModal.tsx`:
- Around line 27-28: The code uses apiOverride (from useApiOverride) directly as
baseUrl (falling back to NEW_API_BASE_URL) and may send the bearer token to
attacker-controlled origins; update DeleteConfirmationModal to validate and
sanitize apiOverride before using it for authenticated requests: allow only
same-origin or a small whitelist of trusted origins (or require a relative
path), reject or ignore overrides that parse to an external origin, and ensure
when an override is rejected you fall back to NEW_API_BASE_URL; apply the same
validation wherever baseUrl is computed from useApiOverride (e.g., the other
baseUrl usages referenced).
🪄 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: CHILL

Plan: Pro

Run ID: cee8c176-93d6-4dfe-bb52-b61884a27536

📥 Commits

Reviewing files that changed from the base of the PR and between f7e799a and 3f3d341.

📒 Files selected for processing (2)
  • app/api/room/delete/route.ts
  • components/Sidebar/Modals/DeleteConfirmationModal.tsx
💤 Files with no reviewable changes (1)
  • app/api/room/delete/route.ts

Comment on lines +27 to +28
const apiOverride = useApiOverride();
const baseUrl = apiOverride || NEW_API_BASE_URL;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Block untrusted API override origins before sending bearer token.

apiOverride is sourced from URL/session storage and directly used as baseUrl for authenticated requests. A crafted ?api= value can exfiltrate bearer tokens to attacker-controlled origins.

🔐 Suggested hardening
+const TRUSTED_API_ORIGINS = new Set([
+  new URL(NEW_API_BASE_URL).origin,
+  // Optional: allow explicit extra origin(s) via env, comma-separated
+  ...(process.env.NEXT_PUBLIC_TRUSTED_API_ORIGINS ?? "")
+    .split(",")
+    .map((v) => v.trim())
+    .filter(Boolean),
+]);
+
 const DeleteConfirmationModal = ({ isOpen, onClose, chatRoom, chatRooms, onDelete }: DeleteConfirmationModalProps) => {
@@
   const handleDelete = async () => {
@@
+    let requestBaseUrl = baseUrl;
+    try {
+      const origin = new URL(requestBaseUrl).origin;
+      if (!TRUSTED_API_ORIGINS.has(origin)) {
+        setError("Invalid API endpoint configuration.");
+        return;
+      }
+    } catch {
+      setError("Invalid API endpoint configuration.");
+      return;
+    }
+
@@
-          const response = await fetch(`${baseUrl}/api/chats`, {
+          const response = await fetch(`${requestBaseUrl}/api/chats`, {
             method: "DELETE",
             headers: {
               "Content-Type": "application/json",
               Authorization: `Bearer ${accessToken}`,
             },
             body: JSON.stringify({ id: roomId }),
           });

As per coding guidelines, "Implement built-in security practices for authentication and data handling."

Also applies to: 76-81

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/Sidebar/Modals/DeleteConfirmationModal.tsx` around lines 27 - 28,
The code uses apiOverride (from useApiOverride) directly as baseUrl (falling
back to NEW_API_BASE_URL) and may send the bearer token to attacker-controlled
origins; update DeleteConfirmationModal to validate and sanitize apiOverride
before using it for authenticated requests: allow only same-origin or a small
whitelist of trusted origins (or require a relative path), reject or ignore
overrides that parse to an external origin, and ensure when an override is
rejected you fall back to NEW_API_BASE_URL; apply the same validation wherever
baseUrl is computed from useApiOverride (e.g., the other baseUrl usages
referenced).

@arpitgupta1214 arpitgupta1214 removed the request for review from sweetmantech March 30, 2026 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant