Skip to content

FIX: API security and client/server implications#67

Open
jayleaton wants to merge 2 commits intomichaelshimeles:mainfrom
jayleaton:main
Open

FIX: API security and client/server implications#67
jayleaton wants to merge 2 commits intomichaelshimeles:mainfrom
jayleaton:main

Conversation

@jayleaton
Copy link
Copy Markdown

@jayleaton jayleaton commented Mar 8, 2026

PLEASE IMPLEMENT URGENTLY!

This template fails to follow multiple web security best practices. If you have a project using this template, then your API is most likely publicly exposed and compromised.

  • Zero API security or user validations. (Middleware can do this but it will become a bloated mess at scale)
  • Client called API routes (Implements server actions as intended for form submissions. I also suggest a larger refactoring to use async server pages for api data fetching.)
  • Add user and database storage for image uploads since its current setup easily allows bad actors to spam fill your cloud storage with random trash. (I suggest a larger refactor to this to handle uploads and updates rather than creating a new one every time.)

IMPORTANT:
This repository has not been updated in several months and might no longer be maintained.
IF this PR has not been merged and you are reading this, I can NOT recommend that people use this template.

I have a monorepo template on my GitHub that uses the same stack as this. It has all of these issues resolved and a few other minor issues that this template has missed.

Note

Require authentication for /api/chat and /api/upload-image, persist uploaded image metadata, and route client uploads through server actions for stronger API security

Introduce createRouteHandler for authenticated API routes, gate /api/chat and /api/upload-image, add userImage persistence, and add uploadImageAction for server-side uploads; update the settings UI and pages to use these paths and session checks. See lib/routeHandler.ts, app/api/upload-image/route.ts, app/api/chat/route.ts, and app/actions/upload.ts.

📍Where to Start

Start with the createRouteHandler in lib/routeHandler.ts, then review the POST handlers in app/api/upload-image/route.ts and app/api/chat/route.ts, followed by uploadImageAction in app/actions/upload.ts.

Macroscope summarized 0d17fb3.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 8, 2026

Greptile Summary

This PR addresses several valid security gaps in the starter kit by introducing createRouteHandler for authenticated API protection, a secureFetch server utility, a userImage DB table for audit tracking, and wiring the settings/upload pages to a new uploadImageAction server action. The core intent is solid and the auth scaffolding (lib/routeHandler.ts) is well-structured.

Key findings:

  • SVG XSS vector: image/svg+xml is included in the upload allowlist. SVG files can embed JavaScript that executes when opened in a browser, making any publicly-accessible R2 URL a stored XSS attack surface. Removing SVG from the allowlist is the simplest fix.
  • Client-controlled MIME type bypass: The MIME type check in app/api/upload-image/route.ts relies solely on the client-provided file.type, which any HTTP client can spoof. Server-side magic-byte validation (e.g. with the file-type npm package) is needed to actually enforce the allowlist.
  • No message validation in the chat route: messages from the request body is forwarded to streamText without length or schema checks, leaving any authenticated user able to escalate OpenAI API costs with oversized payloads.
  • Several issues flagged in prior review threads remain unresolved: the localhost fallback in getBaseUrl(), the Pages Router config export in the App Router upload route, the debug console.log in settings, and the mismatched UI size hints vs. the server's 10 MB cap.

Confidence Score: 2/5

  • Not safe to merge — two active security vulnerabilities (SVG XSS and MIME type bypass) in the upload route need to be resolved first.
  • The authentication scaffolding is a genuine improvement, but the upload route introduces new security issues: SVG files allow stored XSS via the public R2 URL, and the MIME type allowlist can be bypassed by any HTTP client. The chat route also exposes an unguarded cost-escalation path. These need to be fixed before this is production-ready.
  • Pay close attention to app/api/upload-image/route.ts (SVG XSS + MIME bypass) and app/api/chat/route.ts (unvalidated messages input).

Important Files Changed

Filename Overview
lib/routeHandler.ts New authenticated route handler wrapper; clean discriminated-union type design and proper session validation via better-auth.
lib/serverUtils.ts New secureFetch/publicFetch helpers; relies on getBaseUrl() which falls back to localhost in production when NEXT_PUBLIC_APP_URL is unset (flagged in previous thread).
app/api/upload-image/route.ts Authentication added and DB tracking introduced, but SVG XSS and client-controlled MIME type bypass are active security issues; Pages Router config export has no effect in App Router (prior thread).
app/api/chat/route.ts Authentication gating added, but messages array is passed to streamText with no length or schema validation, risking OpenAI cost escalation from any authenticated user.
app/actions/upload.ts New server action that proxies upload to /api/upload-image via HTTP; the loopback anti-pattern was discussed in prior threads with a developer response accepting it as intentional.
db/schema.ts New userImage table added with correct userId foreign key cascade; schema is consistent with existing tables.
middleware.ts Updated comment clarifies API routes are self-protecting; session-cookie check for /dashboard redirect is correct and delegates deep auth validation to createRouteHandler.
app/dashboard/settings/_components/settings-content.tsx Uses uploadImageAction correctly, but retains a debug console.log and shows a 1MB size hint while the server allows 10MB (both flagged in prior threads).
app/dashboard/settings/page.tsx Simple server component with session guard; correctly redirects unauthenticated users before rendering SettingsContent.
app/dashboard/upload/page.tsx Updated to use uploadImageAction; the 5MB client-side limit is inconsistent with the server's 10MB cap (flagged in prior thread) and handleDrop lacks an in-progress guard (noted in PR's filtered issues).
lib/utils.ts getBaseUrl() fallback to localhost will silently break secureFetch in production when NEXT_PUBLIC_APP_URL is unset (flagged in prior thread).

Sequence Diagram

sequenceDiagram
    participant Browser as Browser (Client)
    participant SA as uploadImageAction<br/>(Server Action)
    participant SF as secureFetch<br/>(lib/serverUtils)
    participant RH as createRouteHandler<br/>(lib/routeHandler)
    participant R2 as Cloudflare R2
    participant DB as Database (userImage)

    Browser->>SA: FormData (file)
    SA->>SF: secureFetch('/api/upload-image', { body: formData })
    SF->>SF: auth.api.getSession() — validate session #1
    SF-->>SA: throw Error('Unauthorized') if no session
    SF->>RH: POST /api/upload-image (with cookie)
    RH->>RH: auth.api.getSession() — validate session #2
    RH-->>Browser: 401 Unauthorized if no session
    RH->>RH: Validate file type & size
    RH->>R2: uploadImageAssets(buffer, filename)
    R2-->>RH: public URL
    RH->>DB: db.insert(userImage) { url, filename, userId }
    RH-->>SF: { url }
    SF-->>SA: { url }
    SA-->>Browser: { url }
Loading

Comments Outside Diff (1)

  1. app/api/upload-image/route.ts, line 30-35 (link)

    MIME type check is client-controlled and bypassable

    file.type is a value provided by the client in the Content-Type field of the multipart form-data part. Any HTTP client (e.g. curl, a custom fetch call) can set it to any value regardless of the actual file content. A malicious user could upload an arbitrary binary with Content-Type: image/jpeg and bypass this check entirely.

    To enforce this server-side, read the first few bytes of the file and verify them against known magic byte signatures (e.g. FFD8FF for JPEG, 89504E47 for PNG). A library like file-type can do this in one line:

    import { fileTypeFromBuffer } from 'file-type';
    // ...
    const arrayBuffer = await file.arrayBuffer();
    const buffer = Buffer.from(arrayBuffer);
    const detectedType = await fileTypeFromBuffer(buffer);
    if (!detectedType || !allowedMimeTypes.includes(detectedType.mime)) {
      return NextResponse.json({ error: "Invalid file type." }, { status: 400 });
    }
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: app/api/upload-image/route.ts
    Line: 30-35
    
    Comment:
    **MIME type check is client-controlled and bypassable**
    
    `file.type` is a value provided by the client in the `Content-Type` field of the multipart form-data part. Any HTTP client (e.g. `curl`, a custom `fetch` call) can set it to any value regardless of the actual file content. A malicious user could upload an arbitrary binary with `Content-Type: image/jpeg` and bypass this check entirely.
    
    To enforce this server-side, read the first few bytes of the file and verify them against known magic byte signatures (e.g. `FFD8FF` for JPEG, `89504E47` for PNG). A library like `file-type` can do this in one line:
    
    ```typescript
    import { fileTypeFromBuffer } from 'file-type';
    // ...
    const arrayBuffer = await file.arrayBuffer();
    const buffer = Buffer.from(arrayBuffer);
    const detectedType = await fileTypeFromBuffer(buffer);
    if (!detectedType || !allowedMimeTypes.includes(detectedType.mime)) {
      return NextResponse.json({ error: "Invalid file type." }, { status: 400 });
    }
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 0d17fb3

Comment thread lib/utils.ts
Comment on lines +10 to +11
return `http://localhost:${process.env.PORT ?? 3000}`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

localhost fallback will break production

getBaseUrl() falls back to http://localhost:${port} when NEXT_PUBLIC_APP_URL is not set. secureFetch in lib/serverUtils.ts uses this function to construct the URL for all internal API calls (e.g. the upload action). In any production environment where NEXT_PUBLIC_APP_URL is not configured, every server-action-initiated upload will silently attempt an HTTP request to http://localhost:3000/api/upload-image and fail. This would cause all image uploads to break in production with no obvious error surfacing to the developer.

Consider throwing a clear error if the env var is missing in a non-development environment:

Suggested change
return `http://localhost:${process.env.PORT ?? 3000}`
}
if (process.env.NEXT_PUBLIC_APP_URL) return process.env.NEXT_PUBLIC_APP_URL
if (process.env.NODE_ENV === 'production') {
throw new Error('NEXT_PUBLIC_APP_URL must be set in production')
}
return `http://localhost:${process.env.PORT ?? 3000}`
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/utils.ts
Line: 10-11

Comment:
**`localhost` fallback will break production**

`getBaseUrl()` falls back to `http://localhost:${port}` when `NEXT_PUBLIC_APP_URL` is not set. `secureFetch` in `lib/serverUtils.ts` uses this function to construct the URL for all internal API calls (e.g. the upload action). In any production environment where `NEXT_PUBLIC_APP_URL` is not configured, every server-action-initiated upload will silently attempt an HTTP request to `http://localhost:3000/api/upload-image` and fail. This would cause all image uploads to break in production with no obvious error surfacing to the developer.

Consider throwing a clear error if the env var is missing in a non-development environment:

```suggestion
  if (process.env.NEXT_PUBLIC_APP_URL) return process.env.NEXT_PUBLIC_APP_URL
  if (process.env.NODE_ENV === 'production') {
    throw new Error('NEXT_PUBLIC_APP_URL must be set in production')
  }
  return `http://localhost:${process.env.PORT ?? 3000}`
```

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

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

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.

Comment thread app/api/chat/route.ts Outdated
Comment thread app/dashboard/settings/_components/settings-content.tsx
Comment thread app/dashboard/settings/_components/settings-content.tsx
Comment thread lib/routeHandler.ts
Comment thread lib/serverUtils.ts
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Comment on lines 21 to 35
@@ -32,33 +34,30 @@ export async function POST(req: NextRequest) {
);
}
Copy link
Copy Markdown

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.

Comment thread app/api/chat/route.ts
Comment on lines +10 to +18
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(),
},
});
Copy link
Copy Markdown

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.

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