Skip to content

FIX: API security and client/server implications #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jayleaton wants to merge 2 commits into
base: main
Choose a base branch
from
Open

FIX: API security and client/server implications #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d8c653
fix api security and client/server implications
jayleaton Mar 8, 2026
0d17fb3
Update app/api/chat/route.ts
jayleaton Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/actions/upload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use server'
import { secureFetch } from '@/lib/serverUtils'

export async function uploadImageAction(formData: FormData): Promise<{ url: string }> {
return secureFetch('/api/upload-image', { method: 'POST', body: formData })
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
28 changes: 17 additions & 11 deletions app/api/chat/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import { openai } from "@ai-sdk/openai";
import { streamText } from "ai";
import { createRouteHandler } from "@/lib/routeHandler";
import { NextResponse } from "next/server";

export async function POST(req: Request) {
const { messages } = await req.json();
// IF this is public you might want to add a rate limiter! However I have added authentication checks.
export const POST = createRouteHandler(
{ isAuthenticated: true },
async (req) => {
const { messages } = await req.json();

const result = streamText({
model: openai.responses("gpt-4o"),
messages,
tools: {
web_search_preview: openai.tools.webSearchPreview(),
},
});
const result = streamText({
model: openai.responses("gpt-4o"),
messages,
tools: {
web_search_preview: openai.tools.webSearchPreview(),
},
});
Comment on lines +10 to +18
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 8, 2026

Choose a reason for hiding this comment

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

No validation on messages input — cost-escalation risk

messages is extracted from the request body and forwarded directly to streamText without any schema validation. An authenticated user could send:

  • An arbitrarily large number of messages
  • Extremely long individual message strings
  • Malformed message objects that crash the handler

Because each call consumes OpenAI API credits, this opens the door to deliberate cost escalation by any authenticated user. Consider adding a simple guard:

const { messages } = await req.json();

if (!Array.isArray(messages) || messages.length === 0) {
  return NextResponse.json({ error: "Invalid messages" }, { status: 400 });
}
if (messages.length > 50) {
  return NextResponse.json({ error: "Too many messages" }, { status: 400 });
}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/api/chat/route.ts
Line: 10-18

Comment:
**No validation on `messages` input — cost-escalation risk**

`messages` is extracted from the request body and forwarded directly to `streamText` without any schema validation. An authenticated user could send:
- An arbitrarily large number of messages
- Extremely long individual message strings
- Malformed message objects that crash the handler

Because each call consumes OpenAI API credits, this opens the door to deliberate cost escalation by any authenticated user. Consider adding a simple guard:

```typescript
const { messages } = await req.json();

if (!Array.isArray(messages) || messages.length === 0) {
  return NextResponse.json({ error: "Invalid messages" }, { status: 400 });
}
if (messages.length > 50) {
  return NextResponse.json({ error: "Too many messages" }, { status: 400 });
}
```

How can I resolve this? If you propose a fix, please make it concise.


return result.toDataStreamResponse();
}
return result.toDataStreamResponse() as unknown as NextResponse;
},
);
37 changes: 18 additions & 19 deletions app/api/upload-image/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
import { db } from "@/db/drizzle";
import { userImage } from "@/db/schema";
import { createRouteHandler } from "@/lib/routeHandler";
import { uploadImageAssets } from "@/lib/upload-image";
import { NextRequest, NextResponse } from "next/server";
import { NextResponse } from "next/server";

export const config = {
api: { bodyParser: false }, // Disable default body parsing
api: { bodyParser: false },
};
Comment on lines 7 to 9
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 8, 2026

Choose a reason for hiding this comment

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

Pages Router config has no effect in the App Router

export const config = { api: { bodyParser: false } } is a Next.js Pages Router convention for disabling the built-in body parser. In the App Router (i.e. any file under app/), this export is silently ignored — the runtime already handles FormData natively in route handlers without needing this flag. Keeping it risks confusing future developers into thinking it provides some protection or behavior change.

Suggested change
export const config = {
api: { bodyParser: false }, // Disable default body parsing
api: { bodyParser: false },
};
// App Router handles FormData natively; no bodyParser config needed.

Or simply remove the export entirely.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/api/upload-image/route.ts
Line: 7-9

Comment:
**Pages Router `config` has no effect in the App Router**

`export const config = { api: { bodyParser: false } }` is a Next.js Pages Router convention for disabling the built-in body parser. In the App Router (i.e. any file under `app/`), this export is silently ignored — the runtime already handles `FormData` natively in route handlers without needing this flag. Keeping it risks confusing future developers into thinking it provides some protection or behavior change.

```suggestion
// App Router handles FormData natively; no bodyParser config needed.
```
Or simply remove the export entirely.

How can I resolve this? If you propose a fix, please make it concise.


export async function POST(req: NextRequest) {
try {
// Parse the form data
export const POST = createRouteHandler(
{ isAuthenticated: true },
async (req) => {
const formData = await req.formData();
const file = formData.get("file") as File | null;

if (!file) {
return NextResponse.json({ error: "No file provided" }, { status: 400 });
}

// Validate MIME type - only allow image files
const allowedMimeTypes = [
"image/jpeg",
"image/jpg",
Expand All @@ -32,33 +34,30 @@ export async function POST(req: NextRequest) {
);
}
Comment on lines 21 to 35
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 8, 2026

Choose a reason for hiding this comment

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

SVG files allow stored XSS attacks

image/svg+xml is included in the allowlist. SVG files can contain embedded JavaScript (e.g. <script>alert(1)</script>) that executes when a user opens the file URL directly in a browser. Since uploads are stored on Cloudflare R2 and the URL is returned to any authenticated user, this becomes a stored XSS vector: an authenticated user uploads a malicious SVG, shares the R2 URL, and any victim who opens that link has JavaScript executed in their browser.

Consider removing image/svg+xml from the allowlist:

Suggested change
const allowedMimeTypes = [
"image/jpeg",
"image/jpg",
"image/png",
"image/gif",
"image/webp",
];
Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/api/upload-image/route.ts
Line: 21-35

Comment:
**SVG files allow stored XSS attacks**

`image/svg+xml` is included in the allowlist. SVG files can contain embedded JavaScript (e.g. `<script>alert(1)</script>`) that executes when a user opens the file URL directly in a browser. Since uploads are stored on Cloudflare R2 and the URL is returned to any authenticated user, this becomes a stored XSS vector: an authenticated user uploads a malicious SVG, shares the R2 URL, and any victim who opens that link has JavaScript executed in their browser.

Consider removing `image/svg+xml` from the allowlist:

```suggestion
    const allowedMimeTypes = [
      "image/jpeg",
      "image/jpg",
      "image/png",
      "image/gif",
      "image/webp",
    ];
```

How can I resolve this? If you propose a fix, please make it concise.


// Validate file size - limit to 10MB
const maxSizeInBytes = 10 * 1024 * 1024; // 10MB
const maxSizeInBytes = 10 * 1024 * 1024;
if (file.size > maxSizeInBytes) {
return NextResponse.json(
{ error: "File too large. Maximum size allowed is 10MB." },
{ status: 400 },
);
}

// Convert file to buffer
const arrayBuffer = await file.arrayBuffer();
const buffer = Buffer.from(arrayBuffer);

// Generate a unique filename with original extension
const fileExt = file.name.split(".").pop() || "";
const timestamp = Date.now();
const filename = `upload-${timestamp}.${fileExt || "png"}`;

// Upload the file
const url = await uploadImageAssets(buffer, filename);

await db.insert(userImage).values({
id: crypto.randomUUID(),
url,
filename,
userId: req.user.id,
});

return NextResponse.json({ url });
} catch (error) {
console.error("Upload error:", error);
return NextResponse.json(
{ error: "Failed to process upload" },
{ status: 500 },
);
}
}
},
);
Loading