Security hardening and ForceGraph performance optimization (merged with main) #1219
+518
−35
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CodeAnt AI is reviewing your PR. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThe pull request implements comprehensive security hardening through JWT payload reduction, input validation across multiple API endpoints, error message sanitization, path traversal prevention, and connection lifecycle management. Concurrently, performance optimizations are applied to graph model calculations and ForceGraph rendering via Set-based lookups. A new SECURITY_REVIEW.md document summarizes security fixes, vulnerabilities, and performance gains. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Auth as Auth Handler
participant ConnMap as Connections Map
participant Cleanup as Cleanup Timer
rect rgb(200, 220, 255)
Note over Auth,ConnMap: Existing Connection Reuse (JWT Path)
Client->>Auth: POST /login (existing sessionId)
Auth->>ConnMap: Lookup: connections.get(sessionId)
alt Entry Found
ConnMap-->>Auth: { client, timestamp }
Auth->>Auth: Update entry.timestamp
Auth->>Client: Return JWT + connection reused
else Entry Not Found
ConnMap-->>Auth: null
Auth->>Auth: Fall back to session auth
Auth->>Client: Redirect to session login
end
end
rect rgb(220, 255, 220)
Note over Cleanup,ConnMap: Periodic Cleanup (every 30 min)
Cleanup->>ConnMap: cleanupStaleConnections()
loop For Each Entry
alt Timestamp > 2 hours old
Cleanup->>ConnMap: entry.client.close()
ConnMap->>ConnMap: Delete entry
end
end
end
rect rgb(255, 220, 220)
Note over Client,Auth: API Request with Input Validation
Client->>Auth: GET /api/graph/[graph]/[node]
Auth->>Auth: Validate graphId (non-empty string)
Auth->>Auth: Validate nodeId (non-negative integer)
alt Validation Fails
Auth->>Client: 400 Invalid input
else Validation Passes
Auth->>Auth: Execute query
Auth->>Client: 200 Result
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Note Free review on us!CodeRabbit is offering free reviews until Wed Dec 17 2025 to showcase some of the refinements we've made. Comment |
|
CodeAnt AI finished reviewing your PR. |
…dency updates Co-authored-by: gkorland <[email protected]>
…t, and graph model Co-authored-by: gkorland <[email protected]>
Co-authored-by: gkorland <[email protected]>
Co-authored-by: gkorland <[email protected]>
Copilot
AI
changed the title
[WIP] Review codebase for potential bugs and security issues
Security hardening and ForceGraph performance optimization
Nov 10, 2025
Contributor
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
SECURITY_REVIEW.md (1)
203-223: Add language hints to these fences.markdownlint flags the unnamed code blocks in the testing sections. Tagging them (e.g.,
text) keeps formatting while satisfying the lint rules.-``` +```text ✅ TypeScript Compilation: Success ✅ Next.js Build: Success ✅ ESLint: 0 Errors, 58 Warnings (console.log only)@@
-+text
✅ CodeQL JavaScript Analysis: 0 Alerts
✅ Dependency Audit: 0 Vulnerabilities
✅ No hardcoded credentials found
✅ No dangerous innerHTML usage@@ -``` +```text ✅ Build output size: Stable (no bloat) ✅ No circular dependencies detected ✅ Memory usage: Optimized with connection cleanup</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between fb2e80f8394534f89588c6f9ecffebddacce1705 and ca66f7ac59736868932f5c0ceb6914fc444327d3. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (11)</summary> * `SECURITY_REVIEW.md` (1 hunks) * `app/api/auth/[...nextauth]/options.ts` (5 hunks) * `app/api/auth/login/route.ts` (1 hunks) * `app/api/graph/[graph]/[node]/route.ts` (4 hunks) * `app/api/graph/[graph]/route.ts` (4 hunks) * `app/api/graph/model.ts` (4 hunks) * `app/api/schema/[schema]/[node]/route.ts` (1 hunks) * `app/api/upload/route.ts` (1 hunks) * `app/api/user/[user]/route.ts` (1 hunks) * `app/api/user/route.ts` (5 hunks) * `app/components/ForceGraph.tsx` (6 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>SECURITY_REVIEW.md</summary> 203-203: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 210-210: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 218-218: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>app/api/schema/[schema]/[node]/route.ts (1)</summary><blockquote> `34-58`: **Input validation hardening looks solid.** Good call tightening label and node ID validation before query construction; this closes off the injection vectors without impacting the happy path. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+49
to
+55
| const formData = await request.formData(); | ||
| const file = formData.get("file") as File; | ||
|
|
||
| if (!file) { | ||
| return NextResponse.json({ error: "No files received." }, { status: 400 }); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Reject non-File payloads before calling File APIs.
A crafted multipart request can set file to a simple text field, so file.name.replaceAll(...) throws and we return 500. Gate on instanceof File before using the File methods.
- const formData = await request.formData();
- const file = formData.get("file") as File;
-
- if (!file) {
- return NextResponse.json({ error: "No files received." }, { status: 400 });
- }
+ const formData = await request.formData();
+ const fileEntry = formData.get("file");
+ if (!(fileEntry instanceof File)) {
+ return NextResponse.json({ error: "No files received." }, { status: 400 });
+ }
+ const file = fileEntry;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const formData = await request.formData(); | |
| const file = formData.get("file") as File; | |
| if (!file) { | |
| return NextResponse.json({ error: "No files received." }, { status: 400 }); | |
| } | |
| const formData = await request.formData(); | |
| const fileEntry = formData.get("file"); | |
| if (!(fileEntry instanceof File)) { | |
| return NextResponse.json({ error: "No files received." }, { status: 400 }); | |
| } | |
| const file = fileEntry; |
🤖 Prompt for AI Agents
In app/api/upload/route.ts around lines 49 to 55, the code assumes
formData.get("file") is a File and calls File APIs which will throw for non-File
form fields; before using any File properties or methods, check that the
retrieved value is an instance of File and if not return a 400 JSON error (e.g.,
"No files received" or "Invalid file upload"), ensuring you do not call
file.name.replaceAll or other File methods unless the instanceof check passes.
app/api/user/[user]/route.ts
Outdated
Comment on lines
21
to
41
| // Validate username to prevent injection attacks | ||
| if (!username || typeof username !== 'string' || username.trim().length === 0) { | ||
| return NextResponse.json( | ||
| { message: "Invalid username" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| const roleParam = req.nextUrl.searchParams.get("role"); | ||
| const role = ROLE.get(roleParam || ""); | ||
|
|
||
| try { | ||
| if (!role) throw new Error("Role is missing"); | ||
| if (!role) { | ||
| return NextResponse.json( | ||
| { message: "Invalid or missing role" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| await (await client.connection).aclSetUser(username, role); | ||
| return NextResponse.json({ message: "User created" }, { status: 200 }); | ||
| return NextResponse.json({ message: "User updated" }, { status: 200 }); |
Contributor
There was a problem hiding this comment.
Reapply the username regex here.
The PATCH endpoint still accepts any non-empty string from the path, so characters we explicitly blocked elsewhere (e.g., spaces, quotes) slip through and reach aclSetUser. That reintroduces the injection surface we meant to close. Please enforce the same ^[a-zA-Z0-9_-]+$ check before proceeding.
- if (!username || typeof username !== 'string' || username.trim().length === 0) {
+ if (!username || typeof username !== "string" || username.trim().length === 0) {
return NextResponse.json(
{ message: "Invalid username" },
{ status: 400 }
);
}
+
+ const usernameRegex = /^[a-zA-Z0-9_-]+$/;
+ if (!usernameRegex.test(username)) {
+ return NextResponse.json(
+ { message: "Username can only contain letters, numbers, underscores, and hyphens" },
+ { status: 400 }
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Validate username to prevent injection attacks | |
| if (!username || typeof username !== 'string' || username.trim().length === 0) { | |
| return NextResponse.json( | |
| { message: "Invalid username" }, | |
| { status: 400 } | |
| ); | |
| } | |
| const roleParam = req.nextUrl.searchParams.get("role"); | |
| const role = ROLE.get(roleParam || ""); | |
| try { | |
| if (!role) throw new Error("Role is missing"); | |
| if (!role) { | |
| return NextResponse.json( | |
| { message: "Invalid or missing role" }, | |
| { status: 400 } | |
| ); | |
| } | |
| await (await client.connection).aclSetUser(username, role); | |
| return NextResponse.json({ message: "User created" }, { status: 200 }); | |
| return NextResponse.json({ message: "User updated" }, { status: 200 }); | |
| // Validate username to prevent injection attacks | |
| if (!username || typeof username !== "string" || username.trim().length === 0) { | |
| return NextResponse.json( | |
| { message: "Invalid username" }, | |
| { status: 400 } | |
| ); | |
| } | |
| const usernameRegex = /^[a-zA-Z0-9_-]+$/; | |
| if (!usernameRegex.test(username)) { | |
| return NextResponse.json( | |
| { message: "Username can only contain letters, numbers, underscores, and hyphens" }, | |
| { status: 400 } | |
| ); | |
| } | |
| const roleParam = req.nextUrl.searchParams.get("role"); | |
| const role = ROLE.get(roleParam || ""); | |
| try { | |
| if (!role) { | |
| return NextResponse.json( | |
| { message: "Invalid or missing role" }, | |
| { status: 400 } | |
| ); | |
| } | |
| await (await client.connection).aclSetUser(username, role); | |
| return NextResponse.json({ message: "User updated" }, { status: 200 }); |
🤖 Prompt for AI Agents
In app/api/user/[user]/route.ts around lines 21 to 41, the handler only checks
for a non-empty string username but does not enforce the required character
whitelist; add a strict regex validation using ^[a-zA-Z0-9_-]+$ before calling
aclSetUser and return a 400 NextResponse.json({ message: "Invalid username" })
if it fails. Ensure you trim the username first, confirm it's a string, test
against the regex, and only proceed to parse role and call
client.connection.aclSetUser when the regex passes.
Contributor
|
@copilot please solve conflicts with this pr, and make the ci pass with all the tests |
Resolved conflicts by taking main branch's refactored code structure: - Auth system now uses encrypted token storage - [node] routes renamed to [element] routes - Added validation-body.ts for centralized input validation - Upload route security fixes preserved - SECURITY_REVIEW.md documentation preserved
…ecurity features Co-authored-by: barakb <[email protected]>
Contributor
Author
All merge conflicts resolved and build passing ✅ Merged: origin/main → copilot/review-codebase-for-bugs Resolved Conflicts:
Build Status:
Commit: eb938cd Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Security hardening and ForceGraph performance optimization
Security hardening and ForceGraph performance optimization (merged with main)
Dec 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Comprehensive security review identified critical vulnerabilities and performance bottlenecks in graph rendering. Fixed 8 security issues and optimized ForceGraph algorithms from O(n²) to O(n). Successfully merged with main branch, integrating enhanced authentication and validation systems.
Security Fixes
Critical
Input Validation
^[a-zA-Z0-9_-]+$), 8-char minimum passwordsData Protection
Main Branch Integration
Successfully merged
origin/mainwith the following enhancements:Authentication System
isTokenActive()Validation Framework
validate-body.tswith Zod schemascreateGraphElement,deleteGraphElement,renameGraph,updateUser,createUserAPI Structure
[node]→[element]for consistency_)Merge Details
Performance Optimizations
Algorithm Improvements
ForceGraph Rendering
visibleLinks(),removeLinks(),deleteNeighbors()Memory Management
Results
Security
Performance
Build Status
See
SECURITY_REVIEW.mdfor complete analysis including merge details.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit
New Features
Bug Fixes & Security
Performance
Documentation